From 8d6b0929fb5fb170d4a8543e517b23f793af2fcb Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Fri, 16 Nov 2018 09:12:24 -0800 Subject: [PATCH 1/9] Codespaces as Strings (#2821) --- Gopkg.lock | 3 +- PENDING.md | 2 + baseapp/baseapp.go | 42 +++++++-------- baseapp/baseapp_test.go | 18 ++++--- cmd/gaia/app/app.go | 8 +-- cmd/gaia/cmd/gaiadebug/hack.go | 4 +- docs/_attic/sdk/core/examples/app1.go | 7 +-- docs/_attic/sdk/core/examples/app2.go | 2 +- docs/examples/basecoin/app/app.go | 2 +- docs/examples/democoin/app/app.go | 8 +-- docs/examples/democoin/x/cool/app_test.go | 2 +- docs/examples/democoin/x/cool/errors.go | 2 +- docs/examples/democoin/x/oracle/handler.go | 2 +- docs/examples/democoin/x/pow/app_test.go | 2 +- docs/examples/democoin/x/pow/errors.go | 2 +- .../examples/democoin/x/simplestake/errors.go | 2 +- .../democoin/x/simplestake/handler.go | 4 +- store/rootmultistore_test.go | 15 +++--- types/codespacer.go | 35 ------------- types/codespacer_test.go | 47 ----------------- types/errors.go | 52 +++++-------------- types/errors_test.go | 7 +-- types/result.go | 5 +- types/result_test.go | 2 +- x/auth/ante_test.go | 7 +-- x/bank/app_test.go | 3 +- x/bank/errors.go | 2 +- x/distribution/types/errors.go | 2 +- x/gov/errors.go | 2 +- x/gov/test_common.go | 2 +- x/ibc/app_test.go | 2 +- x/ibc/errors.go | 2 +- x/mock/app_test.go | 4 +- x/mock/test_utils.go | 12 ++--- x/slashing/app_test.go | 7 +-- x/slashing/errors.go | 2 +- x/slashing/handler_test.go | 3 +- x/stake/app_test.go | 2 +- x/stake/types/errors.go | 2 +- 39 files changed, 119 insertions(+), 210 deletions(-) delete mode 100644 types/codespacer.go delete mode 100644 types/codespacer_test.go diff --git a/Gopkg.lock b/Gopkg.lock index bcc306edb..40192b2af 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -165,12 +165,13 @@ version = "v1.2.0" [[projects]] - digest = "1:ea40c24cdbacd054a6ae9de03e62c5f252479b96c716375aace5c120d68647c8" + digest = "1:c0d19ab64b32ce9fe5cf4ddceba78d5bc9807f0016db6b1183599da3dcc24d10" name = "github.com/hashicorp/hcl" packages = [ ".", "hcl/ast", "hcl/parser", + "hcl/printer", "hcl/scanner", "hcl/strconv", "hcl/token", diff --git a/PENDING.md b/PENDING.md index 050646593..a3034c01e 100644 --- a/PENDING.md +++ b/PENDING.md @@ -56,8 +56,10 @@ IMPROVEMENTS - #2773 Require moniker to be provided on `gaiad init`. - #2672 [Makefile] Updated for better Windows compatibility and ledger support logic, get_tools was rewritten as a cross-compatible Makefile. - [#110](https://github.com/tendermint/devops/issues/110) Updated CircleCI job to trigger website build when cosmos docs are updated. + * SDK - [x/mock/simulation] [\#2720] major cleanup, introduction of helper objects, reorganization + - \#2821 Codespaces are now strings * Tendermint - #2796 Update to go-amino 0.14.1 diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 827536d21..34041dff2 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -47,7 +47,6 @@ type BaseApp struct { cms sdk.CommitMultiStore // Main (uncached) state router Router // handle any kind of message queryRouter QueryRouter // router for redirecting query calls - codespacer *sdk.Codespacer // handle module codespacing txDecoder sdk.TxDecoder // unmarshal []byte into sdk.Tx anteHandler sdk.AnteHandler // ante handler for fee and auth @@ -94,13 +93,9 @@ func NewBaseApp(name string, logger log.Logger, db dbm.DB, txDecoder sdk.TxDecod cms: store.NewCommitMultiStore(db), router: NewRouter(), queryRouter: NewQueryRouter(), - codespacer: sdk.NewCodespacer(), txDecoder: txDecoder, } - // Register the undefined & root codespaces, which should not be used by - // any modules. - app.codespacer.RegisterOrPanic(sdk.CodespaceRoot) for _, option := range options { option(app) } @@ -118,11 +113,6 @@ func (app *BaseApp) SetCommitMultiStoreTracer(w io.Writer) { app.cms.WithTracer(w) } -// Register the next available codespace through the baseapp's codespacer, starting from a default -func (app *BaseApp) RegisterCodespace(codespace sdk.CodespaceType) sdk.CodespaceType { - return app.codespacer.RegisterNext(codespace) -} - // Mount IAVL stores to the provided keys in the BaseApp multistore func (app *BaseApp) MountStoresIAVL(keys ...*sdk.KVStoreKey) { for _, key := range keys { @@ -329,8 +319,9 @@ func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) (res abc } case "version": return abci.ResponseQuery{ - Code: uint32(sdk.ABCICodeOK), - Value: []byte(version.GetVersion()), + Code: uint32(sdk.CodeOK), + Codespace: string(sdk.CodespaceRoot), + Value: []byte(version.GetVersion()), } default: result = sdk.ErrUnknownRequest(fmt.Sprintf("Unknown query: %s", path)).Result() @@ -339,8 +330,9 @@ func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) (res abc // Encode with json value := codec.Cdc.MustMarshalBinaryLengthPrefixed(result) return abci.ResponseQuery{ - Code: uint32(sdk.ABCICodeOK), - Value: value, + Code: uint32(sdk.CodeOK), + Codespace: string(sdk.CodespaceRoot), + Value: value, } } msg := "Expected second parameter to be either simulate or version, neither was present" @@ -400,12 +392,13 @@ func handleQueryCustom(app *BaseApp, path []string, req abci.RequestQuery) (res resBytes, err := querier(ctx, path[2:], req) if err != nil { return abci.ResponseQuery{ - Code: uint32(err.ABCICode()), - Log: err.ABCILog(), + Code: uint32(err.Code()), + Codespace: string(err.Codespace()), + Log: err.ABCILog(), } } return abci.ResponseQuery{ - Code: uint32(sdk.ABCICodeOK), + Code: uint32(sdk.CodeOK), Value: resBytes, } } @@ -482,6 +475,7 @@ func (app *BaseApp) DeliverTx(txBytes []byte) (res abci.ResponseDeliverTx) { // Tell the blockchain engine (i.e. Tendermint). return abci.ResponseDeliverTx{ Code: uint32(result.Code), + Codespace: string(result.Codespace), Data: result.Data, Log: result.Log, GasWanted: result.GasWanted, @@ -501,7 +495,6 @@ func validateBasicTxMsgs(msgs []sdk.Msg) sdk.Error { // Validate the Msg. err := msg.ValidateBasic() if err != nil { - err = err.WithDefaultCodespace(sdk.CodespaceRoot) return err } } @@ -526,7 +519,8 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (re logs := make([]string, 0, len(msgs)) var data []byte // NOTE: we just append them all (?!) var tags sdk.Tags // also just append them all - var code sdk.ABCICodeType + var code sdk.CodeType + var codespace sdk.CodespaceType for msgIdx, msg := range msgs { // Match route. msgRoute := msg.Route() @@ -553,6 +547,7 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (re if !msgResult.IsOK() { logs = append(logs, fmt.Sprintf("Msg %d failed: %s", msgIdx, msgResult.Log)) code = msgResult.Code + codespace = msgResult.Codespace break } @@ -562,10 +557,11 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (re // Set the final gas values. result = sdk.Result{ - Code: code, - Data: data, - Log: strings.Join(logs, "\n"), - GasUsed: ctx.GasMeter().GasConsumed(), + Code: code, + Codespace: codespace, + Data: data, + Log: strings.Join(logs, "\n"), + GasUsed: ctx.GasMeter().GasConsumed(), // TODO: FeeAmount/FeeDenom Tags: tags, } diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 9f6414214..35f9fa424 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -678,7 +678,8 @@ func TestRunInvalidTransaction(t *testing.T) { { emptyTx := &txTest{} err := app.Deliver(emptyTx) - require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeInternal), err.Code) + require.EqualValues(t, sdk.CodeInternal, err.Code) + require.EqualValues(t, sdk.CodespaceRoot, err.Codespace) } // Transaction where ValidateBasic fails @@ -701,7 +702,8 @@ func TestRunInvalidTransaction(t *testing.T) { tx := testCase.tx res := app.Deliver(tx) if testCase.fail { - require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeInvalidSequence), res.Code) + require.EqualValues(t, sdk.CodeInvalidSequence, res.Code) + require.EqualValues(t, sdk.CodespaceRoot, res.Codespace) } else { require.True(t, res.IsOK(), fmt.Sprintf("%v", res)) } @@ -712,11 +714,13 @@ func TestRunInvalidTransaction(t *testing.T) { { unknownRouteTx := txTest{[]sdk.Msg{msgNoRoute{}}, 0} err := app.Deliver(unknownRouteTx) - require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeUnknownRequest), err.Code) + require.EqualValues(t, sdk.CodeUnknownRequest, err.Code) + require.EqualValues(t, sdk.CodespaceRoot, err.Codespace) unknownRouteTx = txTest{[]sdk.Msg{msgCounter{}, msgNoRoute{}}, 0} err = app.Deliver(unknownRouteTx) - require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeUnknownRequest), err.Code) + require.EqualValues(t, sdk.CodeUnknownRequest, err.Code) + require.EqualValues(t, sdk.CodespaceRoot, err.Codespace) } // Transaction with an unregistered message @@ -732,7 +736,8 @@ func TestRunInvalidTransaction(t *testing.T) { txBytes, err := newCdc.MarshalBinaryLengthPrefixed(tx) require.NoError(t, err) res := app.DeliverTx(txBytes) - require.EqualValues(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeTxDecode), res.Code) + require.EqualValues(t, sdk.CodeTxDecode, res.Code) + require.EqualValues(t, sdk.CodespaceRoot, res.Codespace) } } @@ -819,7 +824,8 @@ func TestTxGasLimits(t *testing.T) { if !tc.fail { require.True(t, res.IsOK(), fmt.Sprintf("%d: %v, %v", i, tc, res)) } else { - require.Equal(t, res.Code, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeOutOfGas), fmt.Sprintf("%d: %v, %v", i, tc, res)) + require.Equal(t, sdk.CodeOutOfGas, res.Code, fmt.Sprintf("%d: %v, %v", i, tc, res)) + require.Equal(t, sdk.CodespaceRoot, res.Codespace) } } } diff --git a/cmd/gaia/app/app.go b/cmd/gaia/app/app.go index fb8455b5a..c06d82bb1 100644 --- a/cmd/gaia/app/app.go +++ b/cmd/gaia/app/app.go @@ -113,7 +113,7 @@ func NewGaiaApp(logger log.Logger, db dbm.DB, traceStore io.Writer, baseAppOptio app.cdc, app.keyStake, app.tkeyStake, app.bankKeeper, app.paramsKeeper.Subspace(stake.DefaultParamspace), - app.RegisterCodespace(stake.DefaultCodespace), + stake.DefaultCodespace, ) app.mintKeeper = mint.NewKeeper(app.cdc, app.keyMint, app.paramsKeeper.Subspace(mint.DefaultParamspace), @@ -124,19 +124,19 @@ func NewGaiaApp(logger log.Logger, db dbm.DB, traceStore io.Writer, baseAppOptio app.keyDistr, app.paramsKeeper.Subspace(distr.DefaultParamspace), app.bankKeeper, &stakeKeeper, app.feeCollectionKeeper, - app.RegisterCodespace(stake.DefaultCodespace), + distr.DefaultCodespace, ) app.slashingKeeper = slashing.NewKeeper( app.cdc, app.keySlashing, &stakeKeeper, app.paramsKeeper.Subspace(slashing.DefaultParamspace), - app.RegisterCodespace(slashing.DefaultCodespace), + slashing.DefaultCodespace, ) app.govKeeper = gov.NewKeeper( app.cdc, app.keyGov, app.paramsKeeper, app.paramsKeeper.Subspace(gov.DefaultParamspace), app.bankKeeper, &stakeKeeper, - app.RegisterCodespace(gov.DefaultCodespace), + gov.DefaultCodespace, ) // register the staking hooks diff --git a/cmd/gaia/cmd/gaiadebug/hack.go b/cmd/gaia/cmd/gaiadebug/hack.go index 734a83df3..e9f3b5658 100644 --- a/cmd/gaia/cmd/gaiadebug/hack.go +++ b/cmd/gaia/cmd/gaiadebug/hack.go @@ -175,8 +175,8 @@ func NewGaiaApp(logger log.Logger, db dbm.DB, baseAppOptions ...func(*bam.BaseAp // add handlers app.bankKeeper = bank.NewBaseKeeper(app.accountKeeper) app.paramsKeeper = params.NewKeeper(app.cdc, app.keyParams, app.tkeyParams) - app.stakeKeeper = stake.NewKeeper(app.cdc, app.keyStake, app.tkeyStake, app.bankKeeper, app.paramsKeeper.Subspace(stake.DefaultParamspace), app.RegisterCodespace(stake.DefaultCodespace)) - app.slashingKeeper = slashing.NewKeeper(app.cdc, app.keySlashing, app.stakeKeeper, app.paramsKeeper.Subspace(slashing.DefaultParamspace), app.RegisterCodespace(slashing.DefaultCodespace)) + app.stakeKeeper = stake.NewKeeper(app.cdc, app.keyStake, app.tkeyStake, app.bankKeeper, app.paramsKeeper.Subspace(stake.DefaultParamspace), stake.DefaultCodespace) + app.slashingKeeper = slashing.NewKeeper(app.cdc, app.keySlashing, app.stakeKeeper, app.paramsKeeper.Subspace(slashing.DefaultParamspace), slashing.DefaultCodespace) // register message routes app.Router(). diff --git a/docs/_attic/sdk/core/examples/app1.go b/docs/_attic/sdk/core/examples/app1.go index 8ee34fca5..25e8b40de 100644 --- a/docs/_attic/sdk/core/examples/app1.go +++ b/docs/_attic/sdk/core/examples/app1.go @@ -12,7 +12,8 @@ import ( ) const ( - app1Name = "App1" + app1Name = "App1" + bankCodespace = "BANK" ) func NewApp1(logger log.Logger, db dbm.DB) *bapp.BaseApp { @@ -107,7 +108,7 @@ func handleMsgSend(key *sdk.KVStoreKey) sdk.Handler { if !ok { // Create custom error message and return result // Note: Using unreserved error codespace - return sdk.NewError(2, 1, "MsgSend is malformed").Result() + return sdk.NewError(bankCodespace, 1, "MsgSend is malformed").Result() } // Load the store. @@ -137,7 +138,7 @@ func handleFrom(store sdk.KVStore, from sdk.AccAddress, amt sdk.Coins) sdk.Resul accBytes := store.Get(from) if accBytes == nil { // Account was not added to store. Return the result of the error. - return sdk.NewError(2, 101, "Account not added to store").Result() + return sdk.NewError(bankCodespace, 101, "Account not added to store").Result() } // Unmarshal the JSON account bytes. diff --git a/docs/_attic/sdk/core/examples/app2.go b/docs/_attic/sdk/core/examples/app2.go index 24458384c..620a19113 100644 --- a/docs/_attic/sdk/core/examples/app2.go +++ b/docs/_attic/sdk/core/examples/app2.go @@ -126,7 +126,7 @@ func handleMsgIssue(keyIssue *sdk.KVStoreKey, keyAcc *sdk.KVStoreKey) sdk.Handle return func(ctx sdk.Context, msg sdk.Msg) sdk.Result { issueMsg, ok := msg.(MsgIssue) if !ok { - return sdk.NewError(2, 1, "MsgIssue is malformed").Result() + return sdk.NewError(bankCodespace, 1, "MsgIssue is malformed").Result() } // Retrieve stores diff --git a/docs/examples/basecoin/app/app.go b/docs/examples/basecoin/app/app.go index 9515feaad..3e58e71f9 100644 --- a/docs/examples/basecoin/app/app.go +++ b/docs/examples/basecoin/app/app.go @@ -75,7 +75,7 @@ func NewBasecoinApp(logger log.Logger, db dbm.DB, baseAppOptions ...func(*bam.Ba }, ) app.bankKeeper = bank.NewBaseKeeper(app.accountKeeper) - app.ibcMapper = ibc.NewMapper(app.cdc, app.keyIBC, app.RegisterCodespace(ibc.DefaultCodespace)) + app.ibcMapper = ibc.NewMapper(app.cdc, app.keyIBC, ibc.DefaultCodespace) // register message routes app.Router(). diff --git a/docs/examples/democoin/app/app.go b/docs/examples/democoin/app/app.go index bc618e360..ec9db1ae8 100644 --- a/docs/examples/democoin/app/app.go +++ b/docs/examples/democoin/app/app.go @@ -83,10 +83,10 @@ func NewDemocoinApp(logger log.Logger, db dbm.DB) *DemocoinApp { // Add handlers. app.bankKeeper = bank.NewBaseKeeper(app.accountKeeper) - app.coolKeeper = cool.NewKeeper(app.capKeyMainStore, app.bankKeeper, app.RegisterCodespace(cool.DefaultCodespace)) - app.powKeeper = pow.NewKeeper(app.capKeyPowStore, pow.NewConfig("pow", int64(1)), app.bankKeeper, app.RegisterCodespace(pow.DefaultCodespace)) - app.ibcMapper = ibc.NewMapper(app.cdc, app.capKeyIBCStore, app.RegisterCodespace(ibc.DefaultCodespace)) - app.stakeKeeper = simplestake.NewKeeper(app.capKeyStakingStore, app.bankKeeper, app.RegisterCodespace(simplestake.DefaultCodespace)) + app.coolKeeper = cool.NewKeeper(app.capKeyMainStore, app.bankKeeper, cool.DefaultCodespace) + app.powKeeper = pow.NewKeeper(app.capKeyPowStore, pow.NewConfig("pow", int64(1)), app.bankKeeper, pow.DefaultCodespace) + app.ibcMapper = ibc.NewMapper(app.cdc, app.capKeyIBCStore, ibc.DefaultCodespace) + app.stakeKeeper = simplestake.NewKeeper(app.capKeyStakingStore, app.bankKeeper, simplestake.DefaultCodespace) app.Router(). AddRoute("bank", bank.NewHandler(app.bankKeeper)). AddRoute("cool", cool.NewHandler(app.coolKeeper)). diff --git a/docs/examples/democoin/x/cool/app_test.go b/docs/examples/democoin/x/cool/app_test.go index 01cb73ef1..3f1dd6734 100644 --- a/docs/examples/democoin/x/cool/app_test.go +++ b/docs/examples/democoin/x/cool/app_test.go @@ -50,7 +50,7 @@ func getMockApp(t *testing.T) *mock.App { RegisterCodec(mapp.Cdc) keyCool := sdk.NewKVStoreKey("cool") bankKeeper := bank.NewBaseKeeper(mapp.AccountKeeper) - keeper := NewKeeper(keyCool, bankKeeper, mapp.RegisterCodespace(DefaultCodespace)) + keeper := NewKeeper(keyCool, bankKeeper, DefaultCodespace) mapp.Router().AddRoute("cool", NewHandler(keeper)) mapp.SetInitChainer(getInitChainer(mapp, keeper, "ice-cold")) diff --git a/docs/examples/democoin/x/cool/errors.go b/docs/examples/democoin/x/cool/errors.go index 73df2386e..e31d66470 100644 --- a/docs/examples/democoin/x/cool/errors.go +++ b/docs/examples/democoin/x/cool/errors.go @@ -8,7 +8,7 @@ import ( // Cool errors reserve 400 ~ 499. const ( - DefaultCodespace sdk.CodespaceType = 6 + DefaultCodespace sdk.CodespaceType = "cool" // Cool module reserves error 400-499 lawl CodeIncorrectCoolAnswer sdk.CodeType = 400 diff --git a/docs/examples/democoin/x/oracle/handler.go b/docs/examples/democoin/x/oracle/handler.go index c525222c4..fb7587a12 100644 --- a/docs/examples/democoin/x/oracle/handler.go +++ b/docs/examples/democoin/x/oracle/handler.go @@ -94,7 +94,7 @@ func (keeper Keeper) Handle(h Handler, ctx sdk.Context, o Msg, codespace sdk.Cod err := h(cctx, payload) if err != nil { return sdk.Result{ - Code: sdk.ABCICodeOK, + Code: sdk.CodeOK, Log: err.ABCILog(), } } diff --git a/docs/examples/democoin/x/pow/app_test.go b/docs/examples/democoin/x/pow/app_test.go index 5009b7ec5..41901f097 100644 --- a/docs/examples/democoin/x/pow/app_test.go +++ b/docs/examples/democoin/x/pow/app_test.go @@ -27,7 +27,7 @@ func getMockApp(t *testing.T) *mock.App { keyPOW := sdk.NewKVStoreKey("pow") bankKeeper := bank.NewBaseKeeper(mapp.AccountKeeper) config := Config{"pow", 1} - keeper := NewKeeper(keyPOW, config, bankKeeper, mapp.RegisterCodespace(DefaultCodespace)) + keeper := NewKeeper(keyPOW, config, bankKeeper, DefaultCodespace) mapp.Router().AddRoute("pow", keeper.Handler) mapp.SetInitChainer(getInitChainer(mapp, keeper)) diff --git a/docs/examples/democoin/x/pow/errors.go b/docs/examples/democoin/x/pow/errors.go index e25964da7..8368c2359 100644 --- a/docs/examples/democoin/x/pow/errors.go +++ b/docs/examples/democoin/x/pow/errors.go @@ -9,7 +9,7 @@ type CodeType = sdk.CodeType // POW errors reserve 200 ~ 299 const ( - DefaultCodespace sdk.CodespaceType = 5 + DefaultCodespace sdk.CodespaceType = "pow" CodeInvalidDifficulty CodeType = 201 CodeNonexistentDifficulty CodeType = 202 CodeNonexistentReward CodeType = 203 diff --git a/docs/examples/democoin/x/simplestake/errors.go b/docs/examples/democoin/x/simplestake/errors.go index 8125e57aa..a0fc6e1ac 100644 --- a/docs/examples/democoin/x/simplestake/errors.go +++ b/docs/examples/democoin/x/simplestake/errors.go @@ -6,7 +6,7 @@ import ( // simple stake errors reserve 300 ~ 399. const ( - DefaultCodespace sdk.CodespaceType = 4 + DefaultCodespace sdk.CodespaceType = moduleName // simplestake errors reserve 300 - 399. CodeEmptyValidator sdk.CodeType = 300 diff --git a/docs/examples/democoin/x/simplestake/handler.go b/docs/examples/democoin/x/simplestake/handler.go index 114f06643..104058eec 100644 --- a/docs/examples/democoin/x/simplestake/handler.go +++ b/docs/examples/democoin/x/simplestake/handler.go @@ -22,12 +22,12 @@ func handleMsgBond() sdk.Result { // Removed ValidatorSet from result because it does not get used. // TODO: Implement correct bond/unbond handling return sdk.Result{ - Code: sdk.ABCICodeOK, + Code: sdk.CodeOK, } } func handleMsgUnbond() sdk.Result { return sdk.Result{ - Code: sdk.ABCICodeOK, + Code: sdk.CodeOK, } } diff --git a/store/rootmultistore_test.go b/store/rootmultistore_test.go index 501c5b730..cd555d6f2 100644 --- a/store/rootmultistore_test.go +++ b/store/rootmultistore_test.go @@ -156,34 +156,37 @@ func TestMultiStoreQuery(t *testing.T) { // Test bad path. query := abci.RequestQuery{Path: "/key", Data: k, Height: ver} qres := multi.Query(query) - require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeUnknownRequest), sdk.ABCICodeType(qres.Code)) + require.EqualValues(t, sdk.CodeUnknownRequest, qres.Code) + require.EqualValues(t, sdk.CodespaceRoot, qres.Codespace) query.Path = "h897fy32890rf63296r92" qres = multi.Query(query) - require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeUnknownRequest), sdk.ABCICodeType(qres.Code)) + require.EqualValues(t, sdk.CodeUnknownRequest, qres.Code) + require.EqualValues(t, sdk.CodespaceRoot, qres.Codespace) // Test invalid store name. query.Path = "/garbage/key" qres = multi.Query(query) - require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeUnknownRequest), sdk.ABCICodeType(qres.Code)) + require.EqualValues(t, sdk.CodeUnknownRequest, qres.Code) + require.EqualValues(t, sdk.CodespaceRoot, qres.Codespace) // Test valid query with data. query.Path = "/store1/key" qres = multi.Query(query) - require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeOK), sdk.ABCICodeType(qres.Code)) + require.EqualValues(t, sdk.CodeOK, qres.Code) require.Equal(t, v, qres.Value) // Test valid but empty query. query.Path = "/store2/key" query.Prove = true qres = multi.Query(query) - require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeOK), sdk.ABCICodeType(qres.Code)) + require.EqualValues(t, sdk.CodeOK, qres.Code) require.Nil(t, qres.Value) // Test store2 data. query.Data = k2 qres = multi.Query(query) - require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeOK), sdk.ABCICodeType(qres.Code)) + require.EqualValues(t, sdk.CodeOK, qres.Code) require.Equal(t, v2, qres.Value) } diff --git a/types/codespacer.go b/types/codespacer.go deleted file mode 100644 index 92025c1a6..000000000 --- a/types/codespacer.go +++ /dev/null @@ -1,35 +0,0 @@ -package types - -// Codespacer is a simple struct to track reserved codespaces -type Codespacer struct { - reserved map[CodespaceType]bool -} - -// NewCodespacer generates a new Codespacer with the starting codespace -func NewCodespacer() *Codespacer { - return &Codespacer{ - reserved: make(map[CodespaceType]bool), - } -} - -// RegisterNext reserves and returns the next available codespace, starting from a default, and panics if the maximum codespace is reached -func (c *Codespacer) RegisterNext(codespace CodespaceType) CodespaceType { - for { - if !c.reserved[codespace] { - c.reserved[codespace] = true - return codespace - } - codespace++ - if codespace == MaximumCodespace { - panic("Maximum codespace reached!") - } - } -} - -// RegisterOrPanic reserved a codespace or panics if it is unavailable -func (c *Codespacer) RegisterOrPanic(codespace CodespaceType) { - if c.reserved[codespace] { - panic("Cannot register codespace, already reserved") - } - c.reserved[codespace] = true -} diff --git a/types/codespacer_test.go b/types/codespacer_test.go deleted file mode 100644 index 7052aa92a..000000000 --- a/types/codespacer_test.go +++ /dev/null @@ -1,47 +0,0 @@ -package types - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func TestRegisterNext(t *testing.T) { - codespacer := NewCodespacer() - // unregistered, allow - code1 := codespacer.RegisterNext(CodespaceType(2)) - require.Equal(t, code1, CodespaceType(2)) - // registered, pick next - code2 := codespacer.RegisterNext(CodespaceType(2)) - require.Equal(t, code2, CodespaceType(3)) - // pick next - code3 := codespacer.RegisterNext(CodespaceType(2)) - require.Equal(t, code3, CodespaceType(4)) - // skip 1 - code4 := codespacer.RegisterNext(CodespaceType(6)) - require.Equal(t, code4, CodespaceType(6)) - code5 := codespacer.RegisterNext(CodespaceType(2)) - require.Equal(t, code5, CodespaceType(5)) - code6 := codespacer.RegisterNext(CodespaceType(2)) - require.Equal(t, code6, CodespaceType(7)) - // panic on maximum - defer func() { - r := recover() - require.NotNil(t, r, "Did not panic on maximum codespace") - }() - codespacer.RegisterNext(MaximumCodespace - 1) - codespacer.RegisterNext(MaximumCodespace - 1) -} - -func TestRegisterOrPanic(t *testing.T) { - codespacer := NewCodespacer() - // unregistered, allow - code1 := codespacer.RegisterNext(CodespaceType(2)) - require.Equal(t, code1, CodespaceType(2)) - // panic on duplicate - defer func() { - r := recover() - require.NotNil(t, r, "Did not panic on duplicate codespace") - }() - codespacer.RegisterOrPanic(CodespaceType(2)) -} diff --git a/types/errors.go b/types/errors.go index 965a97c87..46436531e 100644 --- a/types/errors.go +++ b/types/errors.go @@ -10,37 +10,22 @@ import ( abci "github.com/tendermint/tendermint/abci/types" ) -// ABCICodeType - combined codetype / codespace -type ABCICodeType uint32 - -// CodeType - code identifier within codespace -type CodeType uint16 +// CodeType - ABCI code identifier within codespace +type CodeType uint32 // CodespaceType - codespace identifier -type CodespaceType uint16 +type CodespaceType string // IsOK - is everything okay? -func (code ABCICodeType) IsOK() bool { - if code == ABCICodeOK { +func (code CodeType) IsOK() bool { + if code == CodeOK { return true } return false } -// get the abci code from the local code and codespace -func ToABCICode(space CodespaceType, code CodeType) ABCICodeType { - // TODO: Make Tendermint more aware of codespaces. - if space == CodespaceRoot && code == CodeOK { - return ABCICodeOK - } - return ABCICodeType((uint32(space) << 16) | uint32(code)) -} - // SDK error codes const ( - // ABCI error codes - ABCICodeOK ABCICodeType = 0 - // Base error codes CodeOK CodeType = 0 CodeInternal CodeType = 1 @@ -62,11 +47,8 @@ const ( // CodespaceRoot is a codespace for error codes in this file only. // Notice that 0 is an "unset" codespace, which can be overridden with // Error.WithDefaultCodespace(). - CodespaceUndefined CodespaceType = 0 - CodespaceRoot CodespaceType = 1 - - // Maximum reservable codespace (2^16 - 1) - MaximumCodespace CodespaceType = 65535 + CodespaceUndefined CodespaceType = "" + CodespaceRoot CodespaceType = "sdk" ) func unknownCodeMsg(code CodeType) string { @@ -185,7 +167,6 @@ type Error interface { Code() CodeType Codespace() CodespaceType ABCILog() string - ABCICode() ABCICodeType Result() Result QueryResult() abci.ResponseQuery } @@ -239,17 +220,12 @@ func (err *sdkError) TraceSDK(format string, args ...interface{}) Error { // Implements ABCIError. func (err *sdkError) Error() string { return fmt.Sprintf(`ERROR: -Codespace: %d +Codespace: %s Code: %d Message: %#v `, err.codespace, err.code, err.cmnError.Error()) } -// Implements ABCIError. -func (err *sdkError) ABCICode() ABCICodeType { - return ToABCICode(err.codespace, err.code) -} - // Implements Error. func (err *sdkError) Codespace() CodespaceType { return err.codespace @@ -267,7 +243,6 @@ func (err *sdkError) ABCILog() string { jsonErr := humanReadableError{ Codespace: err.codespace, Code: err.code, - ABCICode: err.ABCICode(), Message: errMsg, } bz, er := cdc.MarshalJSON(jsonErr) @@ -280,16 +255,18 @@ func (err *sdkError) ABCILog() string { func (err *sdkError) Result() Result { return Result{ - Code: err.ABCICode(), - Log: err.ABCILog(), + Code: err.Code(), + Codespace: err.Codespace(), + Log: err.ABCILog(), } } // QueryResult allows us to return sdk.Error.QueryResult() in query responses func (err *sdkError) QueryResult() abci.ResponseQuery { return abci.ResponseQuery{ - Code: uint32(err.ABCICode()), - Log: err.ABCILog(), + Code: uint32(err.Code()), + Codespace: string(err.Codespace()), + Log: err.ABCILog(), } } @@ -324,6 +301,5 @@ func mustGetMsgIndex(abciLog string) int { type humanReadableError struct { Codespace CodespaceType `json:"codespace"` Code CodeType `json:"code"` - ABCICode ABCICodeType `json:"abci_code"` Message string `json:"message"` } diff --git a/types/errors_test.go b/types/errors_test.go index 1d63e0990..8d4f88226 100644 --- a/types/errors_test.go +++ b/types/errors_test.go @@ -42,7 +42,7 @@ var errFns = []errFn{ } func TestCodeType(t *testing.T) { - require.True(t, ABCICodeOK.IsOK()) + require.True(t, CodeOK.IsOK()) for tcnum, c := range codeTypes { msg := CodeToDefaultMsg(c) @@ -59,12 +59,9 @@ func TestErrFn(t *testing.T) { codeType := codeTypes[i] require.Equal(t, err.Code(), codeType, "Err function expected to return proper code. tc #%d", i) require.Equal(t, err.Codespace(), CodespaceRoot, "Err function expected to return proper codespace. tc #%d", i) - require.Equal(t, err.Result().Code, ToABCICode(CodespaceRoot, codeType), "Err function expected to return proper ABCICode. tc #%d") - require.Equal(t, err.QueryResult().Code, uint32(err.ABCICode()), "Err function expected to return proper ABCICode from QueryResult. tc #%d") + require.Equal(t, err.QueryResult().Code, uint32(err.Code()), "Err function expected to return proper Code from QueryResult. tc #%d") require.Equal(t, err.QueryResult().Log, err.ABCILog(), "Err function expected to return proper ABCILog from QueryResult. tc #%d") } - - require.Equal(t, ABCICodeOK, ToABCICode(CodespaceRoot, CodeOK)) } func TestAppendMsgToErr(t *testing.T) { diff --git a/types/result.go b/types/result.go index 63b8a6e18..20893a076 100644 --- a/types/result.go +++ b/types/result.go @@ -4,7 +4,10 @@ package types type Result struct { // Code is the response code, is stored back on the chain. - Code ABCICodeType + Code CodeType + + // Codespace is the string referring to the domain of an error + Codespace CodespaceType // Data is any data returned from the app. Data []byte diff --git a/types/result_test.go b/types/result_test.go index e0305932c..9765933d9 100644 --- a/types/result_test.go +++ b/types/result_test.go @@ -13,6 +13,6 @@ func TestResult(t *testing.T) { res.Data = []byte("data") require.True(t, res.IsOK()) - res.Code = ABCICodeType(1) + res.Code = CodeType(1) require.False(t, res.IsOK()) } diff --git a/x/auth/ante_test.go b/x/auth/ante_test.go index 98725a74b..c376d2996 100644 --- a/x/auth/ante_test.go +++ b/x/auth/ante_test.go @@ -43,7 +43,7 @@ func privAndAddr() (crypto.PrivKey, sdk.AccAddress) { func checkValidTx(t *testing.T, anteHandler sdk.AnteHandler, ctx sdk.Context, tx sdk.Tx, simulate bool) { _, result, abort := anteHandler(ctx, tx, simulate) require.False(t, abort) - require.Equal(t, sdk.ABCICodeOK, result.Code) + require.Equal(t, sdk.CodeOK, result.Code) require.True(t, result.IsOK()) } @@ -51,8 +51,9 @@ func checkValidTx(t *testing.T, anteHandler sdk.AnteHandler, ctx sdk.Context, tx func checkInvalidTx(t *testing.T, anteHandler sdk.AnteHandler, ctx sdk.Context, tx sdk.Tx, simulate bool, code sdk.CodeType) { newCtx, result, abort := anteHandler(ctx, tx, simulate) require.True(t, abort) - require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, code), result.Code, - fmt.Sprintf("Expected %v, got %v", sdk.ToABCICode(sdk.CodespaceRoot, code), result)) + + require.Equal(t, code, result.Code, fmt.Sprintf("Expected %v, got %v", code, result)) + require.Equal(t, sdk.CodespaceRoot, result.Codespace) if code == sdk.CodeOutOfGas { stdTx, ok := tx.(StdTx) diff --git a/x/bank/app_test.go b/x/bank/app_test.go index 906573eae..c71a9b392 100644 --- a/x/bank/app_test.go +++ b/x/bank/app_test.go @@ -144,7 +144,8 @@ func TestMsgSendWithAccounts(t *testing.T) { tx.Signatures[0].Sequence = 1 res := mapp.Deliver(tx) - require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeUnauthorized), res.Code, res.Log) + require.EqualValues(t, sdk.CodeUnauthorized, res.Code, res.Log) + require.EqualValues(t, sdk.CodespaceRoot, res.Codespace) // resigning the tx with the bumped sequence should work mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{sendMsg1, sendMsg2}, []int64{0}, []int64{1}, true, true, priv1) diff --git a/x/bank/errors.go b/x/bank/errors.go index cf11ddc22..5df5cfdce 100644 --- a/x/bank/errors.go +++ b/x/bank/errors.go @@ -7,7 +7,7 @@ import ( // Bank errors reserve 100 ~ 199. const ( - DefaultCodespace sdk.CodespaceType = 2 + DefaultCodespace sdk.CodespaceType = "bank" CodeInvalidInput sdk.CodeType = 101 CodeInvalidOutput sdk.CodeType = 102 diff --git a/x/distribution/types/errors.go b/x/distribution/types/errors.go index 605c1b38d..36de11f74 100644 --- a/x/distribution/types/errors.go +++ b/x/distribution/types/errors.go @@ -8,7 +8,7 @@ import ( type CodeType = sdk.CodeType const ( - DefaultCodespace sdk.CodespaceType = 6 + DefaultCodespace sdk.CodespaceType = "DISTR" CodeInvalidInput CodeType = 103 CodeNoDistributionInfo CodeType = 104 ) diff --git a/x/gov/errors.go b/x/gov/errors.go index e803d080e..9b9aa25db 100644 --- a/x/gov/errors.go +++ b/x/gov/errors.go @@ -8,7 +8,7 @@ import ( ) const ( - DefaultCodespace sdk.CodespaceType = 5 + DefaultCodespace sdk.CodespaceType = "GOV" CodeUnknownProposal sdk.CodeType = 1 CodeInactiveProposal sdk.CodeType = 2 diff --git a/x/gov/test_common.go b/x/gov/test_common.go index a0bc80fc2..67b9fd902 100644 --- a/x/gov/test_common.go +++ b/x/gov/test_common.go @@ -35,7 +35,7 @@ func getMockApp(t *testing.T, numGenAccs int) (*mock.App, Keeper, stake.Keeper, pk := params.NewKeeper(mapp.Cdc, keyGlobalParams, tkeyGlobalParams) ck := bank.NewBaseKeeper(mapp.AccountKeeper) - sk := stake.NewKeeper(mapp.Cdc, keyStake, tkeyStake, ck, pk.Subspace(stake.DefaultParamspace), mapp.RegisterCodespace(stake.DefaultCodespace)) + sk := stake.NewKeeper(mapp.Cdc, keyStake, tkeyStake, ck, pk.Subspace(stake.DefaultParamspace), stake.DefaultCodespace) keeper := NewKeeper(mapp.Cdc, keyGov, pk, pk.Subspace("testgov"), ck, sk, DefaultCodespace) mapp.Router().AddRoute("gov", NewHandler(keeper)) diff --git a/x/ibc/app_test.go b/x/ibc/app_test.go index 9b360c7c2..e59588a5a 100644 --- a/x/ibc/app_test.go +++ b/x/ibc/app_test.go @@ -20,7 +20,7 @@ func getMockApp(t *testing.T) *mock.App { RegisterCodec(mapp.Cdc) keyIBC := sdk.NewKVStoreKey("ibc") - ibcMapper := NewMapper(mapp.Cdc, keyIBC, mapp.RegisterCodespace(DefaultCodespace)) + ibcMapper := NewMapper(mapp.Cdc, keyIBC, DefaultCodespace) bankKeeper := bank.NewBaseKeeper(mapp.AccountKeeper) mapp.Router().AddRoute("ibc", NewHandler(ibcMapper, bankKeeper)) diff --git a/x/ibc/errors.go b/x/ibc/errors.go index 7a3194baf..96ae58066 100644 --- a/x/ibc/errors.go +++ b/x/ibc/errors.go @@ -6,7 +6,7 @@ import ( // IBC errors reserve 200 ~ 299. const ( - DefaultCodespace sdk.CodespaceType = 3 + DefaultCodespace sdk.CodespaceType = "ibc" // IBC errors reserve 200 - 299. CodeInvalidSequence sdk.CodeType = 200 diff --git a/x/mock/app_test.go b/x/mock/app_test.go index 9e12cac8e..c47ddb717 100644 --- a/x/mock/app_test.go +++ b/x/mock/app_test.go @@ -71,7 +71,9 @@ func TestCheckAndDeliverGenTx(t *testing.T) { []int64{accs[1].GetAccountNumber()}, []int64{accs[1].GetSequence() + 1}, true, false, privKeys[1], ) - require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeUnauthorized), res.Code, res.Log) + + require.Equal(t, sdk.CodeUnauthorized, res.Code, res.Log) + require.Equal(t, sdk.CodespaceRoot, res.Codespace) // Resigning the tx with the correct privKey should result in an OK result SignCheckDeliver( diff --git a/x/mock/test_utils.go b/x/mock/test_utils.go index 4e60478fa..130339d0e 100644 --- a/x/mock/test_utils.go +++ b/x/mock/test_utils.go @@ -57,9 +57,9 @@ func CheckGenTx( res := app.Check(tx) if expPass { - require.Equal(t, sdk.ABCICodeOK, res.Code, res.Log) + require.Equal(t, sdk.CodeOK, res.Code, res.Log) } else { - require.NotEqual(t, sdk.ABCICodeOK, res.Code, res.Log) + require.NotEqual(t, sdk.CodeOK, res.Code, res.Log) } return res @@ -78,9 +78,9 @@ func SignCheckDeliver( res := app.Simulate(tx) if expSimPass { - require.Equal(t, sdk.ABCICodeOK, res.Code, res.Log) + require.Equal(t, sdk.CodeOK, res.Code, res.Log) } else { - require.NotEqual(t, sdk.ABCICodeOK, res.Code, res.Log) + require.NotEqual(t, sdk.CodeOK, res.Code, res.Log) } // Simulate a sending a transaction and committing a block @@ -88,9 +88,9 @@ func SignCheckDeliver( res = app.Deliver(tx) if expPass { - require.Equal(t, sdk.ABCICodeOK, res.Code, res.Log) + require.Equal(t, sdk.CodeOK, res.Code, res.Log) } else { - require.NotEqual(t, sdk.ABCICodeOK, res.Code, res.Log) + require.NotEqual(t, sdk.CodeOK, res.Code, res.Log) } app.EndBlock(abci.RequestEndBlock{}) diff --git a/x/slashing/app_test.go b/x/slashing/app_test.go index dd96ff51e..bfcbcba0b 100644 --- a/x/slashing/app_test.go +++ b/x/slashing/app_test.go @@ -36,8 +36,8 @@ func getMockApp(t *testing.T) (*mock.App, stake.Keeper, Keeper) { bankKeeper := bank.NewBaseKeeper(mapp.AccountKeeper) paramsKeeper := params.NewKeeper(mapp.Cdc, keyParams, tkeyParams) - stakeKeeper := stake.NewKeeper(mapp.Cdc, keyStake, tkeyStake, bankKeeper, paramsKeeper.Subspace(stake.DefaultParamspace), mapp.RegisterCodespace(stake.DefaultCodespace)) - keeper := NewKeeper(mapp.Cdc, keySlashing, stakeKeeper, paramsKeeper.Subspace(DefaultParamspace), mapp.RegisterCodespace(DefaultCodespace)) + stakeKeeper := stake.NewKeeper(mapp.Cdc, keyStake, tkeyStake, bankKeeper, paramsKeeper.Subspace(stake.DefaultParamspace), stake.DefaultCodespace) + keeper := NewKeeper(mapp.Cdc, keySlashing, stakeKeeper, paramsKeeper.Subspace(DefaultParamspace), DefaultCodespace) mapp.Router().AddRoute("stake", stake.NewHandler(stakeKeeper)) mapp.Router().AddRoute("slashing", NewHandler(keeper)) @@ -126,5 +126,6 @@ func TestSlashingMsgs(t *testing.T) { // unjail should fail with unknown validator res := mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{unjailMsg}, []int64{0}, []int64{1}, false, false, priv1) - require.Equal(t, sdk.ToABCICode(DefaultCodespace, CodeValidatorNotJailed), res.Code) + require.EqualValues(t, CodeValidatorNotJailed, res.Code) + require.EqualValues(t, DefaultCodespace, res.Codespace) } diff --git a/x/slashing/errors.go b/x/slashing/errors.go index 77cb2d28e..0fa523c25 100644 --- a/x/slashing/errors.go +++ b/x/slashing/errors.go @@ -10,7 +10,7 @@ type CodeType = sdk.CodeType const ( // Default slashing codespace - DefaultCodespace sdk.CodespaceType = 10 + DefaultCodespace sdk.CodespaceType = "SLASH" CodeInvalidValidator CodeType = 101 CodeValidatorJailed CodeType = 102 diff --git a/x/slashing/handler_test.go b/x/slashing/handler_test.go index ef307547c..9041bfe56 100644 --- a/x/slashing/handler_test.go +++ b/x/slashing/handler_test.go @@ -26,7 +26,8 @@ func TestCannotUnjailUnlessJailed(t *testing.T) { // assert non-jailed validator can't be unjailed got = slh(ctx, NewMsgUnjail(addr)) require.False(t, got.IsOK(), "allowed unjail of non-jailed validator") - require.Equal(t, sdk.ToABCICode(DefaultCodespace, CodeValidatorNotJailed), got.Code) + require.EqualValues(t, CodeValidatorNotJailed, got.Code) + require.EqualValues(t, DefaultCodespace, got.Codespace) } func TestJailedValidatorDelegations(t *testing.T) { diff --git a/x/stake/app_test.go b/x/stake/app_test.go index b24018a6c..2866acf1d 100644 --- a/x/stake/app_test.go +++ b/x/stake/app_test.go @@ -29,7 +29,7 @@ func getMockApp(t *testing.T) (*mock.App, Keeper) { bankKeeper := bank.NewBaseKeeper(mApp.AccountKeeper) pk := params.NewKeeper(mApp.Cdc, keyParams, tkeyParams) - keeper := NewKeeper(mApp.Cdc, keyStake, tkeyStake, bankKeeper, pk.Subspace(DefaultParamspace), mApp.RegisterCodespace(DefaultCodespace)) + keeper := NewKeeper(mApp.Cdc, keyStake, tkeyStake, bankKeeper, pk.Subspace(DefaultParamspace), DefaultCodespace) mApp.Router().AddRoute("stake", NewHandler(keeper)) mApp.SetEndBlocker(getEndBlocker(keeper)) diff --git a/x/stake/types/errors.go b/x/stake/types/errors.go index 1a6ed6a64..bbef528d5 100644 --- a/x/stake/types/errors.go +++ b/x/stake/types/errors.go @@ -11,7 +11,7 @@ import ( type CodeType = sdk.CodeType const ( - DefaultCodespace sdk.CodespaceType = 4 + DefaultCodespace sdk.CodespaceType = "STAKE" CodeInvalidValidator CodeType = 101 CodeInvalidDelegation CodeType = 102 From 15b6fa0959206cb5d61cca80b0b567bc771bf024 Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Fri, 16 Nov 2018 13:33:47 -0500 Subject: [PATCH 2/9] Cache-wrap context during ante handler exec (#2781) * Use cache-wrapped multi-store in ante * Implement TestBaseAppAnteHandler * Add reference documentation for BaseApp/CheckTx/DeliverTx --- PENDING.md | 7 ++- baseapp/baseapp.go | 69 ++++++++++++++------- baseapp/baseapp_test.go | 109 +++++++++++++++++++++++++++++++--- cmd/gaia/cli_test/cli_test.go | 4 ++ docs/reference/baseapp.md | 68 ++++++++++++++++++--- 5 files changed, 214 insertions(+), 43 deletions(-) diff --git a/PENDING.md b/PENDING.md index a3034c01e..6295d5572 100644 --- a/PENDING.md +++ b/PENDING.md @@ -53,9 +53,10 @@ IMPROVEMENTS * [\#2749](https://github.com/cosmos/cosmos-sdk/pull/2749) Add --chain-id flag to gaiad testnet * Gaia - - #2773 Require moniker to be provided on `gaiad init`. - - #2672 [Makefile] Updated for better Windows compatibility and ledger support logic, get_tools was rewritten as a cross-compatible Makefile. - - [#110](https://github.com/tendermint/devops/issues/110) Updated CircleCI job to trigger website build when cosmos docs are updated. + - #2772 Update BaseApp to not persist state when the ante handler fails on DeliverTx. + - #2773 Require moniker to be provided on `gaiad init`. + - #2672 [Makefile] Updated for better Windows compatibility and ledger support logic, get_tools was rewritten as a cross-compatible Makefile. + - [#110](https://github.com/tendermint/devops/issues/110) Updated CircleCI job to trigger website build when cosmos docs are updated. * SDK - [x/mock/simulation] [\#2720] major cleanup, introduction of helper objects, reorganization diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 34041dff2..0d5571024 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -10,7 +10,6 @@ import ( abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto/tmhash" - cmn "github.com/tendermint/tendermint/libs/common" dbm "github.com/tendermint/tendermint/libs/db" "github.com/tendermint/tendermint/libs/log" @@ -505,11 +504,11 @@ func validateBasicTxMsgs(msgs []sdk.Msg) sdk.Error { // retrieve the context for the ante handler and store the tx bytes; store // the vote infos if the tx runs within the deliverTx() state. func (app *BaseApp) getContextForAnte(mode runTxMode, txBytes []byte) (ctx sdk.Context) { - // Get the context - ctx = getState(app, mode).ctx.WithTxBytes(txBytes) + ctx = app.getState(mode).ctx.WithTxBytes(txBytes) if mode == runTxModeDeliver { ctx = ctx.WithVoteInfos(app.voteInfos) } + return } @@ -571,7 +570,7 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (re // Returns the applicantion's deliverState if app is in runTxModeDeliver, // otherwise it returns the application's checkstate. -func getState(app *BaseApp, mode runTxMode) *state { +func (app *BaseApp) getState(mode runTxMode) *state { if mode == runTxModeCheck || mode == runTxModeSimulate { return app.checkState } @@ -581,20 +580,42 @@ func getState(app *BaseApp, mode runTxMode) *state { func (app *BaseApp) initializeContext(ctx sdk.Context, mode runTxMode) sdk.Context { if mode == runTxModeSimulate { - ctx = ctx.WithMultiStore(getState(app, runTxModeSimulate).CacheMultiStore()) + ctx = ctx.WithMultiStore(app.getState(runTxModeSimulate).CacheMultiStore()) } return ctx } +// cacheTxContext returns a new context based off of the provided context with a +// cache wrapped multi-store and the store itself to allow the caller to write +// changes from the cached multi-store. +func (app *BaseApp) cacheTxContext( + ctx sdk.Context, txBytes []byte, mode runTxMode, +) (sdk.Context, sdk.CacheMultiStore) { + + msCache := app.getState(mode).CacheMultiStore() + if msCache.TracingEnabled() { + msCache = msCache.WithTracingContext( + sdk.TraceContext( + map[string]interface{}{ + "txHash": fmt.Sprintf("%X", tmhash.Sum(txBytes)), + }, + ), + ).(sdk.CacheMultiStore) + } + + return ctx.WithMultiStore(msCache), msCache +} + // runTx processes a transaction. The transactions is proccessed via an -// anteHandler. txBytes may be nil in some cases, eg. in tests. Also, in the -// future we may support "internal" transactions. +// anteHandler. The provided txBytes may be nil in some cases, eg. in tests. For +// further details on transaction execution, reference the BaseApp SDK +// documentation. func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk.Result) { // 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 msCache sdk.CacheMultiStore + ctx := app.getContextForAnte(mode, txBytes) ctx = app.initializeContext(ctx, mode) @@ -619,16 +640,27 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk return err.Result() } - // run the ante handler + // Execute the ante handler if one is defined. if app.anteHandler != nil { - newCtx, result, abort := app.anteHandler(ctx, tx, (mode == runTxModeSimulate)) + var anteCtx sdk.Context + var msCache sdk.CacheMultiStore + + // Cache wrap context before anteHandler call in case it aborts. + // This is required for both CheckTx and DeliverTx. + // https://github.com/cosmos/cosmos-sdk/issues/2772 + // NOTE: Alternatively, we could require that anteHandler ensures that + // writes do not happen if aborted/failed. This may have some + // performance benefits, but it'll be more difficult to get right. + anteCtx, msCache = app.cacheTxContext(ctx, txBytes, mode) + + newCtx, result, abort := app.anteHandler(anteCtx, tx, (mode == runTxModeSimulate)) if abort { return result } if !newCtx.IsZero() { ctx = newCtx } - + msCache.Write() gasWanted = result.GasWanted } @@ -638,17 +670,10 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk return } - // Keep the state in a transient CacheWrap in case processing the messages - // fails. - msCache = getState(app, mode).CacheMultiStore() - if msCache.TracingEnabled() { - msCache = msCache.WithTracingContext(sdk.TraceContext( - map[string]interface{}{"txHash": cmn.HexBytes(tmhash.Sum(txBytes)).String()}, - )).(sdk.CacheMultiStore) - } - - ctx = ctx.WithMultiStore(msCache) - result = app.runMsgs(ctx, msgs, mode) + // Create a new context based off of the existing context with a cache wrapped + // multi-store in case message processing fails. + runMsgCtx, msCache := app.cacheTxContext(ctx, txBytes, mode) + result = app.runMsgs(runMsgCtx, msgs, mode) result.GasWanted = gasWanted // only update state if all messages pass diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 35f9fa424..9c2e7e1a0 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -282,8 +282,19 @@ func TestInitChainer(t *testing.T) { // Simple tx with a list of Msgs. type txTest struct { - Msgs []sdk.Msg - Counter int64 + Msgs []sdk.Msg + Counter int64 + FailOnAnte bool +} + +func (tx *txTest) setFailOnAnte(fail bool) { + tx.FailOnAnte = fail +} + +func (tx *txTest) setFailOnHandler(fail bool) { + for i, msg := range tx.Msgs { + tx.Msgs[i] = msgCounter{msg.(msgCounter).Counter, fail} + } } // Implements Tx @@ -297,7 +308,8 @@ const ( // ValidateBasic() fails on negative counters. // Otherwise it's up to the handlers type msgCounter struct { - Counter int64 + Counter int64 + FailOnHandler bool } // Implements Msg @@ -315,9 +327,9 @@ func (msg msgCounter) ValidateBasic() sdk.Error { func newTxCounter(txInt int64, msgInts ...int64) *txTest { var msgs []sdk.Msg for _, msgInt := range msgInts { - msgs = append(msgs, msgCounter{msgInt}) + msgs = append(msgs, msgCounter{msgInt, false}) } - return &txTest{msgs, txInt} + return &txTest{msgs, txInt, false} } // a msg we dont know how to route @@ -369,8 +381,13 @@ func testTxDecoder(cdc *codec.Codec) sdk.TxDecoder { func anteHandlerTxTest(t *testing.T, capKey *sdk.KVStoreKey, storeKey []byte) sdk.AnteHandler { return func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) { store := ctx.KVStore(capKey) - msgCounter := tx.(txTest).Counter - res = incrementingCounter(t, store, storeKey, msgCounter) + txTest := tx.(txTest) + + if txTest.FailOnAnte { + return newCtx, sdk.ErrInternal("ante handler failure").Result(), true + } + + res = incrementingCounter(t, store, storeKey, txTest.Counter) return } } @@ -381,10 +398,15 @@ func handlerMsgCounter(t *testing.T, capKey *sdk.KVStoreKey, deliverKey []byte) var msgCount int64 switch m := msg.(type) { case *msgCounter: + if m.FailOnHandler { + return sdk.ErrInternal("message handler failure").Result() + } + msgCount = m.Counter case *msgCounter2: msgCount = m.Counter } + return incrementingCounter(t, store, deliverKey, msgCount) } } @@ -712,12 +734,12 @@ func TestRunInvalidTransaction(t *testing.T) { // Transaction with no known route { - unknownRouteTx := txTest{[]sdk.Msg{msgNoRoute{}}, 0} + unknownRouteTx := txTest{[]sdk.Msg{msgNoRoute{}}, 0, false} err := app.Deliver(unknownRouteTx) require.EqualValues(t, sdk.CodeUnknownRequest, err.Code) require.EqualValues(t, sdk.CodespaceRoot, err.Codespace) - unknownRouteTx = txTest{[]sdk.Msg{msgCounter{}, msgNoRoute{}}, 0} + unknownRouteTx = txTest{[]sdk.Msg{msgCounter{}, msgNoRoute{}}, 0, false} err = app.Deliver(unknownRouteTx) require.EqualValues(t, sdk.CodeUnknownRequest, err.Code) require.EqualValues(t, sdk.CodespaceRoot, err.Codespace) @@ -829,3 +851,72 @@ func TestTxGasLimits(t *testing.T) { } } } + +func TestBaseAppAnteHandler(t *testing.T) { + anteKey := []byte("ante-key") + anteOpt := func(bapp *BaseApp) { + bapp.SetAnteHandler(anteHandlerTxTest(t, capKey1, anteKey)) + } + + deliverKey := []byte("deliver-key") + routerOpt := func(bapp *BaseApp) { + bapp.Router().AddRoute(routeMsgCounter, handlerMsgCounter(t, capKey1, deliverKey)) + } + + cdc := codec.New() + app := setupBaseApp(t, anteOpt, routerOpt) + + app.InitChain(abci.RequestInitChain{}) + registerTestCodec(cdc) + app.BeginBlock(abci.RequestBeginBlock{}) + + // execute a tx that will fail ante handler execution + // + // NOTE: State should not be mutated here. This will be implicitly checked by + // the next txs ante handler execution (anteHandlerTxTest). + tx := newTxCounter(0, 0) + tx.setFailOnAnte(true) + txBytes, err := cdc.MarshalBinaryLengthPrefixed(tx) + require.NoError(t, err) + res := app.DeliverTx(txBytes) + require.False(t, res.IsOK(), fmt.Sprintf("%v", res)) + + ctx := app.getState(runTxModeDeliver).ctx + store := ctx.KVStore(capKey1) + require.Equal(t, int64(0), getIntFromStore(store, anteKey)) + + // execute at tx that will pass the ante handler (the checkTx state should + // mutate) but will fail the message handler + tx = newTxCounter(0, 0) + tx.setFailOnHandler(true) + + txBytes, err = cdc.MarshalBinaryLengthPrefixed(tx) + require.NoError(t, err) + + res = app.DeliverTx(txBytes) + require.False(t, res.IsOK(), fmt.Sprintf("%v", res)) + + ctx = app.getState(runTxModeDeliver).ctx + store = ctx.KVStore(capKey1) + require.Equal(t, int64(1), getIntFromStore(store, anteKey)) + require.Equal(t, int64(0), getIntFromStore(store, deliverKey)) + + // execute a successful ante handler and message execution where state is + // implicitly checked by previous tx executions + tx = newTxCounter(1, 0) + + txBytes, err = cdc.MarshalBinaryLengthPrefixed(tx) + require.NoError(t, err) + + res = app.DeliverTx(txBytes) + require.True(t, res.IsOK(), fmt.Sprintf("%v", res)) + + ctx = app.getState(runTxModeDeliver).ctx + store = ctx.KVStore(capKey1) + require.Equal(t, int64(2), getIntFromStore(store, anteKey)) + require.Equal(t, int64(1), getIntFromStore(store, deliverKey)) + + // commit + app.EndBlock(abci.RequestEndBlock{}) + app.Commit() +} diff --git a/cmd/gaia/cli_test/cli_test.go b/cmd/gaia/cli_test/cli_test.go index 874c2a29a..62f4d0d64 100644 --- a/cmd/gaia/cli_test/cli_test.go +++ b/cmd/gaia/cli_test/cli_test.go @@ -537,16 +537,20 @@ func TestGaiaCLISendGenerateSignAndBroadcast(t *testing.T) { success, stdout, _ = executeWriteRetStdStreams(t, fmt.Sprintf( "gaiacli tx broadcast %v --json %v", flags, signedTxFile.Name())) require.True(t, success) + var result struct { Response abci.ResponseDeliverTx } + 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) + tests.WaitForNextNBlocksTM(2, port) barAcc := executeGetAccount(t, fmt.Sprintf("gaiacli query account %s %v", barAddr, flags)) require.Equal(t, int64(10), barAcc.GetCoins().AmountOf(stakeTypes.DefaultBondDenom).Int64()) + fooAcc = executeGetAccount(t, fmt.Sprintf("gaiacli query account %s %v", fooAddr, flags)) require.Equal(t, int64(40), fooAcc.GetCoins().AmountOf(stakeTypes.DefaultBondDenom).Int64()) } diff --git a/docs/reference/baseapp.md b/docs/reference/baseapp.md index 1828021c2..e1a80e293 100644 --- a/docs/reference/baseapp.md +++ b/docs/reference/baseapp.md @@ -1,13 +1,63 @@ -# baseApp +# BaseApp -`baseApp` requires stores to be mounted via capabilities keys - handlers can only access stores they're given the key to. The `baseApp` ensures all stores are properly loaded, cached, and committed. One mounted store is considered the "main" - it holds the latest block header, from which we can find and load the most recent state. +The BaseApp defines the foundational implementation for a basic ABCI application +so that your Cosmos-SDK application can communicate with an underlying +Tendermint node. -`baseApp` distinguishes between two handler types: `AnteHandler` and `MsgHandler`. Whilst the former is a global validity check that applies to all transactions from all modules, i.e. it checks nonces and whether balances are sufficient to pay fees, validates signatures and ensures that transactions don't carry too many signatures, the latter is the full state transition function. -During CheckTx the state transition function is only applied to the checkTxState and should return -before any expensive state transitions are run (this is up to each developer). It also needs to return the estimated -gas cost. +The BaseApp is composed of many internal components. Some of the most important +include the `CommitMultiStore` and its internal state. The internal state is +essentially two sub-states, both of which are used for transaction execution +during different phases, `CheckTx` and `DeliverTx` respectively. During block +commitment, only the `DeliverTx` is persisted. -During DeliverTx the state transition function is applied to the blockchain state and the transactions -need to be fully executed. +The BaseApp requires stores to be mounted via capabilities keys - handlers can +only access stores they're given the key to. The `baseApp` ensures all stores are +properly loaded, cached, and committed. One mounted store is considered the +"main" - it holds the latest block header, from which we can find and load the +most recent state. -BaseApp is responsible for managing the context passed into handlers - it makes the block header available and provides the right stores for CheckTx and DeliverTx. BaseApp is completely agnostic to serialization formats. \ No newline at end of file +The BaseApp distinguishes between two handler types - the `AnteHandler` and the +`MsgHandler`. The former is a global validity check (checking nonces, sigs and +sufficient balances to pay fees, e.g. things that apply to all transaction from +all modules), the later is the full state transition function. + +During `CheckTx` the state transition function is only applied to the `checkTxState` +and should return before any expensive state transitions are run +(this is up to each developer). It also needs to return the estimated gas cost. + +During `DeliverTx` the state transition function is applied to the blockchain +state and the transactions need to be fully executed. + +The BaseApp is responsible for managing the context passed into handlers - +it makes the block header available and provides the right stores for `CheckTx` +and `DeliverTx`. BaseApp is completely agnostic to serialization formats. + +## Transaction Life Cycle + +During the execution of a transaction, it may pass through both `CheckTx` and +`DeliverTx` as defined in the ABCI specification. `CheckTx` is executed by the +proposing validator and is used for the Tendermint mempool for all full nodes. + +Both `CheckTx` and `DeliverTx` execute the application's AnteHandler (if +defined), where the AnteHandler is responsible for pre-message validation +checks such as account and signature validation, fee deduction and collection, +and incrementing sequence numbers. + +### CheckTx + +During the execution of `CheckTx`, only the AnteHandler is executed. + +State transitions due to the AnteHandler are persisted between subsequent calls +of `CheckTx` in the check-tx state, unless the AnteHandler fails and aborts. + +### DeliverTx + +During the execution of `DeliverTx`, the AnteHandler and Handler is executed. + +The transaction execution during `DeliverTx` operates in a similar fashion to +`CheckTx`. However, state transitions that occur during the AnteHandler are +persisted even when the following Handler processing logic fails. + +It is possible that a malicious proposer may include a transaction in a block +that fails the AnteHandler. In this case, all state transitions for the +offending transaction are discarded. From 9676ce7d48e61c20b4371e113c53e27d1b8d0cf9 Mon Sep 17 00:00:00 2001 From: Jack Zampolin Date: Fri, 16 Nov 2018 14:21:36 -0800 Subject: [PATCH 3/9] Expose LCD router, allowing devs to register custom routes from their modules (#2836) * Fixes #1081 --- PENDING.md | 1 + client/flags.go | 65 +++-- client/keys/utils.go | 15 +- client/lcd/root.go | 268 ++++++++++----------- client/lcd/test_helpers.go | 36 ++- cmd/gaia/cmd/gaiacli/main.go | 44 +++- docs/examples/basecoin/cmd/basecli/main.go | 28 ++- docs/examples/democoin/cmd/democli/main.go | 22 +- 8 files changed, 300 insertions(+), 179 deletions(-) diff --git a/PENDING.md b/PENDING.md index 6295d5572..a8f278ebb 100644 --- a/PENDING.md +++ b/PENDING.md @@ -48,6 +48,7 @@ FEATURES IMPROVEMENTS * Gaia REST API (`gaiacli advanced rest-server`) + * [\#2836](https://github.com/cosmos/cosmos-sdk/pull/2836) Expose LCD router to allow users to register routes there. * Gaia CLI (`gaiacli`) * [\#2749](https://github.com/cosmos/cosmos-sdk/pull/2749) Add --chain-id flag to gaiad testnet diff --git a/client/flags.go b/client/flags.go index 9d03939e0..dfadab07e 100644 --- a/client/flags.go +++ b/client/flags.go @@ -17,25 +17,32 @@ const ( DefaultGasLimit = 200000 GasFlagSimulate = "simulate" - FlagUseLedger = "ledger" - FlagChainID = "chain-id" - FlagNode = "node" - FlagHeight = "height" - FlagGas = "gas" - FlagGasAdjustment = "gas-adjustment" - FlagTrustNode = "trust-node" - FlagFrom = "from" - FlagName = "name" - FlagAccountNumber = "account-number" - FlagSequence = "sequence" - FlagMemo = "memo" - FlagFee = "fee" - FlagAsync = "async" - FlagJson = "json" - FlagPrintResponse = "print-response" - FlagDryRun = "dry-run" - FlagGenerateOnly = "generate-only" - FlagIndentResponse = "indent" + FlagUseLedger = "ledger" + FlagChainID = "chain-id" + FlagNode = "node" + FlagHeight = "height" + FlagGas = "gas" + FlagGasAdjustment = "gas-adjustment" + FlagTrustNode = "trust-node" + FlagFrom = "from" + FlagName = "name" + FlagAccountNumber = "account-number" + FlagSequence = "sequence" + FlagMemo = "memo" + FlagFee = "fee" + FlagAsync = "async" + FlagJson = "json" + FlagPrintResponse = "print-response" + FlagDryRun = "dry-run" + FlagGenerateOnly = "generate-only" + FlagIndentResponse = "indent" + FlagListenAddr = "laddr" + FlagCORS = "cors" + FlagMaxOpenConnections = "max-open" + FlagInsecure = "insecure" + FlagSSLHosts = "ssl-hosts" + FlagSSLCertFile = "ssl-certfile" + FlagSSLKeyFile = "ssl-keyfile" ) // LineBreak can be included in a command list to provide a blank line @@ -92,6 +99,26 @@ func PostCommands(cmds ...*cobra.Command) []*cobra.Command { return cmds } +// RegisterRestServerFlags registers the flags required for rest server +func RegisterRestServerFlags(cmd *cobra.Command) *cobra.Command { + cmd.Flags().String(FlagListenAddr, "tcp://localhost:1317", "The address for the server to listen on") + cmd.Flags().Bool(FlagInsecure, false, "Do not set up SSL/TLS layer") + cmd.Flags().String(FlagSSLHosts, "", "Comma-separated hostnames and IPs to generate a certificate for") + cmd.Flags().String(FlagSSLCertFile, "", "Path to a SSL certificate file. If not supplied, a self-signed certificate will be generated.") + cmd.Flags().String(FlagSSLKeyFile, "", "Path to a key file; ignored if a certificate file is not supplied.") + cmd.Flags().String(FlagCORS, "", "Set the domains that can make CORS requests (* for all)") + cmd.Flags().String(FlagChainID, "", "Chain ID of Tendermint node") + cmd.Flags().String(FlagNode, "tcp://localhost:26657", "Address of the node to connect to") + cmd.Flags().Int(FlagMaxOpenConnections, 1000, "The number of maximum open connections") + cmd.Flags().Bool(FlagTrustNode, false, "Trust connected full node (don't verify proofs for responses)") + cmd.Flags().Bool(FlagIndentResponse, false, "Add indent to JSON response") + + viper.BindPFlag(FlagTrustNode, cmd.Flags().Lookup(FlagTrustNode)) + viper.BindPFlag(FlagChainID, cmd.Flags().Lookup(FlagChainID)) + viper.BindPFlag(FlagNode, cmd.Flags().Lookup(FlagNode)) + return cmd +} + // Gas flag parsing functions // GasSetting encapsulates the possible values passed through the --gas flag. diff --git a/client/keys/utils.go b/client/keys/utils.go index 742b512d3..0ac4a9c90 100644 --- a/client/keys/utils.go +++ b/client/keys/utils.go @@ -2,20 +2,17 @@ package keys import ( "fmt" - "github.com/syndtr/goleveldb/leveldb/opt" + "net/http" "path/filepath" - "github.com/spf13/viper" - + "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/crypto/keys" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/spf13/viper" + "github.com/syndtr/goleveldb/leveldb/opt" "github.com/tendermint/tendermint/libs/cli" dbm "github.com/tendermint/tendermint/libs/db" - - "github.com/cosmos/cosmos-sdk/client" - - "github.com/cosmos/cosmos-sdk/codec" - sdk "github.com/cosmos/cosmos-sdk/types" - "net/http" ) // KeyDBName is the directory under root where we store the keys diff --git a/client/lcd/root.go b/client/lcd/root.go index 8366b6114..3d6181646 100644 --- a/client/lcd/root.go +++ b/client/lcd/root.go @@ -10,15 +10,9 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/context" "github.com/cosmos/cosmos-sdk/client/keys" - "github.com/cosmos/cosmos-sdk/client/rpc" - "github.com/cosmos/cosmos-sdk/client/tx" "github.com/cosmos/cosmos-sdk/codec" + keybase "github.com/cosmos/cosmos-sdk/crypto/keys" "github.com/cosmos/cosmos-sdk/server" - auth "github.com/cosmos/cosmos-sdk/x/auth/client/rest" - bank "github.com/cosmos/cosmos-sdk/x/bank/client/rest" - gov "github.com/cosmos/cosmos-sdk/x/gov/client/rest" - slashing "github.com/cosmos/cosmos-sdk/x/slashing/client/rest" - stake "github.com/cosmos/cosmos-sdk/x/stake/client/rest" "github.com/gorilla/mux" "github.com/rakyll/statik/fs" "github.com/spf13/cobra" @@ -27,159 +21,155 @@ import ( tmserver "github.com/tendermint/tendermint/rpc/lib/server" ) -const ( - flagListenAddr = "laddr" - flagCORS = "cors" - flagMaxOpenConnections = "max-open" - flagInsecure = "insecure" - flagSSLHosts = "ssl-hosts" - flagSSLCertFile = "ssl-certfile" - flagSSLKeyFile = "ssl-keyfile" -) +// RestServer represents the Light Client Rest server +type RestServer struct { + Mux *mux.Router + CliCtx context.CLIContext + KeyBase keybase.Keybase + Cdc *codec.Codec + + log log.Logger + listener net.Listener + fingerprint string +} + +// NewRestServer creates a new rest server instance +func NewRestServer(cdc *codec.Codec) *RestServer { + r := mux.NewRouter() + cliCtx := context.NewCLIContext().WithCodec(cdc) + + // Register version methods on the router + r.HandleFunc("/version", CLIVersionRequestHandler).Methods("GET") + r.HandleFunc("/node_version", NodeVersionRequestHandler(cliCtx)).Methods("GET") + + logger := log.NewTMLogger(log.NewSyncWriter(os.Stdout)).With("module", "rest-server") + + return &RestServer{ + Mux: r, + CliCtx: cliCtx, + Cdc: cdc, + + log: logger, + } +} + +func (rs *RestServer) setKeybase(kb keybase.Keybase) { + // If a keybase is passed in, set it and return + if kb != nil { + rs.KeyBase = kb + return + } + + // Otherwise get the keybase and set it + kb, err := keys.GetKeyBase() //XXX + if err != nil { + fmt.Printf("Failed to open Keybase: %s, exiting...", err) + os.Exit(1) + } + rs.KeyBase = kb +} + +// Start starts the rest server +func (rs *RestServer) Start(listenAddr string, sslHosts string, + certFile string, keyFile string, maxOpen int, insecure bool) (err error) { + + server.TrapSignal(func() { + err := rs.listener.Close() + rs.log.Error("error closing listener", "err", err) + }) + + // TODO: re-enable insecure mode once #2715 has been addressed + if insecure { + fmt.Println( + "Insecure mode is temporarily disabled, please locally generate an " + + "SSL certificate to test. Support will be re-enabled soon!", + ) + // listener, err = tmserver.StartHTTPServer( + // listenAddr, handler, logger, + // tmserver.Config{MaxOpenConnections: maxOpen}, + // ) + // if err != nil { + // return + // } + } else { + if certFile != "" { + // validateCertKeyFiles() is needed to work around tendermint/tendermint#2460 + err = validateCertKeyFiles(certFile, keyFile) + if err != nil { + return err + } + + // cert/key pair is provided, read the fingerprint + rs.fingerprint, err = fingerprintFromFile(certFile) + if err != nil { + return err + } + } else { + // if certificate is not supplied, generate a self-signed one + certFile, keyFile, rs.fingerprint, err = genCertKeyFilesAndReturnFingerprint(sslHosts) + if err != nil { + return err + } + + defer func() { + os.Remove(certFile) + os.Remove(keyFile) + }() + } + + rs.listener, err = tmserver.StartHTTPAndTLSServer( + listenAddr, rs.Mux, + certFile, keyFile, + rs.log, + tmserver.Config{MaxOpenConnections: maxOpen}, + ) + if err != nil { + return + } + + rs.log.Info(rs.fingerprint) + rs.log.Info("REST server started") + } + + // logger.Info("REST server started") + + return nil +} // ServeCommand will generate a long-running rest server // (aka Light Client Daemon) that exposes functionality similar // to the cli, but over rest -func ServeCommand(cdc *codec.Codec) *cobra.Command { - +func (rs *RestServer) ServeCommand() *cobra.Command { cmd := &cobra.Command{ Use: "rest-server", Short: "Start LCD (light-client daemon), a local REST server", RunE: func(cmd *cobra.Command, args []string) (err error) { - listenAddr := viper.GetString(flagListenAddr) - handler := createHandler(cdc) + rs.setKeybase(nil) + // Start the rest server and return error if one exists + err = rs.Start( + viper.GetString(client.FlagListenAddr), + viper.GetString(client.FlagSSLHosts), + viper.GetString(client.FlagSSLCertFile), + viper.GetString(client.FlagSSLKeyFile), + viper.GetInt(client.FlagMaxOpenConnections), + viper.GetBool(client.FlagInsecure)) - registerSwaggerUI(handler) - - logger := log.NewTMLogger(log.NewSyncWriter(os.Stdout)).With("module", "rest-server") - maxOpen := viper.GetInt(flagMaxOpenConnections) - sslHosts := viper.GetString(flagSSLHosts) - certFile := viper.GetString(flagSSLCertFile) - keyFile := viper.GetString(flagSSLKeyFile) - - var listener net.Listener - var fingerprint string - - server.TrapSignal(func() { - err := listener.Close() - logger.Error("error closing listener", "err", err) - }) - - var cleanupFunc func() - - // TODO: re-enable insecure mode once #2715 has been addressed - if viper.GetBool(flagInsecure) { - fmt.Println( - "Insecure mode is temporarily disabled, please locally generate an " + - "SSL certificate to test. Support will be re-enabled soon!", - ) - // listener, err = tmserver.StartHTTPServer( - // listenAddr, handler, logger, - // tmserver.Config{MaxOpenConnections: maxOpen}, - // ) - // if err != nil { - // return - // } - } else { - if certFile != "" { - // validateCertKeyFiles() is needed to work around tendermint/tendermint#2460 - err = validateCertKeyFiles(certFile, keyFile) - if err != nil { - return err - } - - // cert/key pair is provided, read the fingerprint - fingerprint, err = fingerprintFromFile(certFile) - if err != nil { - return err - } - } else { - // if certificate is not supplied, generate a self-signed one - certFile, keyFile, fingerprint, err = genCertKeyFilesAndReturnFingerprint(sslHosts) - if err != nil { - return err - } - - cleanupFunc = func() { - os.Remove(certFile) - os.Remove(keyFile) - } - - defer cleanupFunc() - } - - listener, err = tmserver.StartHTTPAndTLSServer( - listenAddr, handler, - certFile, keyFile, - logger, - tmserver.Config{MaxOpenConnections: maxOpen}, - ) - if err != nil { - return - } - - logger.Info(fingerprint) - logger.Info("REST server started") - } - - // logger.Info("REST server started") - - return nil + return err }, } - cmd.Flags().String(flagListenAddr, "tcp://localhost:1317", "The address for the server to listen on") - cmd.Flags().Bool(flagInsecure, false, "Do not set up SSL/TLS layer") - cmd.Flags().String(flagSSLHosts, "", "Comma-separated hostnames and IPs to generate a certificate for") - cmd.Flags().String(flagSSLCertFile, "", "Path to a SSL certificate file. If not supplied, a self-signed certificate will be generated.") - cmd.Flags().String(flagSSLKeyFile, "", "Path to a key file; ignored if a certificate file is not supplied.") - cmd.Flags().String(flagCORS, "", "Set the domains that can make CORS requests (* for all)") - cmd.Flags().String(client.FlagChainID, "", "Chain ID of Tendermint node") - cmd.Flags().String(client.FlagNode, "tcp://localhost:26657", "Address of the node to connect to") - cmd.Flags().Int(flagMaxOpenConnections, 1000, "The number of maximum open connections") - cmd.Flags().Bool(client.FlagTrustNode, false, "Trust connected full node (don't verify proofs for responses)") - cmd.Flags().Bool(client.FlagIndentResponse, false, "Add indent to JSON response") - - viper.BindPFlag(client.FlagTrustNode, cmd.Flags().Lookup(client.FlagTrustNode)) - viper.BindPFlag(client.FlagChainID, cmd.Flags().Lookup(client.FlagChainID)) - viper.BindPFlag(client.FlagNode, cmd.Flags().Lookup(client.FlagNode)) + client.RegisterRestServerFlags(cmd) return cmd } -func createHandler(cdc *codec.Codec) *mux.Router { - r := mux.NewRouter() - - kb, err := keys.GetKeyBase() //XXX - if err != nil { - panic(err) - } - - cliCtx := context.NewCLIContext().WithCodec(cdc) - - // TODO: make more functional? aka r = keys.RegisterRoutes(r) - r.HandleFunc("/version", CLIVersionRequestHandler).Methods("GET") - r.HandleFunc("/node_version", NodeVersionRequestHandler(cliCtx)).Methods("GET") - - keys.RegisterRoutes(r, cliCtx.Indent) - rpc.RegisterRoutes(cliCtx, r) - tx.RegisterRoutes(cliCtx, r, cdc) - auth.RegisterRoutes(cliCtx, r, cdc, "acc") - bank.RegisterRoutes(cliCtx, r, cdc, kb) - stake.RegisterRoutes(cliCtx, r, cdc, kb) - slashing.RegisterRoutes(cliCtx, r, cdc, kb) - gov.RegisterRoutes(cliCtx, r, cdc) - - return r -} - -func registerSwaggerUI(r *mux.Router) { +func (rs *RestServer) registerSwaggerUI() { statikFS, err := fs.New() if err != nil { panic(err) } staticServer := http.FileServer(statikFS) - r.PathPrefix("/swagger-ui/").Handler(http.StripPrefix("/swagger-ui/", staticServer)) + rs.Mux.PathPrefix("/swagger-ui/").Handler(http.StripPrefix("/swagger-ui/", staticServer)) } func validateCertKeyFiles(certFile, keyFile string) error { diff --git a/client/lcd/test_helpers.go b/client/lcd/test_helpers.go index 774fd4d6a..4fbacf3d5 100644 --- a/client/lcd/test_helpers.go +++ b/client/lcd/test_helpers.go @@ -19,6 +19,8 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/keys" + "github.com/cosmos/cosmos-sdk/client/rpc" + "github.com/cosmos/cosmos-sdk/client/tx" gapp "github.com/cosmos/cosmos-sdk/cmd/gaia/app" "github.com/cosmos/cosmos-sdk/codec" crkeys "github.com/cosmos/cosmos-sdk/crypto/keys" @@ -45,6 +47,12 @@ import ( "github.com/tendermint/tendermint/proxy" tmrpc "github.com/tendermint/tendermint/rpc/lib/server" tmtypes "github.com/tendermint/tendermint/types" + + authRest "github.com/cosmos/cosmos-sdk/x/auth/client/rest" + bankRest "github.com/cosmos/cosmos-sdk/x/bank/client/rest" + govRest "github.com/cosmos/cosmos-sdk/x/gov/client/rest" + slashingRest "github.com/cosmos/cosmos-sdk/x/slashing/client/rest" + stakeRest "github.com/cosmos/cosmos-sdk/x/stake/client/rest" ) // makePathname creates a unique pathname for each test. It will panic if it @@ -103,6 +111,13 @@ func GetKeyBase(t *testing.T) crkeys.Keybase { return keybase } +// GetTestKeyBase fetches the current testing keybase +func GetTestKeyBase(t *testing.T) crkeys.Keybase { + keybase, err := keys.GetKeyBaseWithWritePerm() + require.NoError(t, err) + return keybase +} + // CreateAddr adds an address to the key store and returns an address and seed. // It also requires that the key could be created. func CreateAddr(t *testing.T, name, password string, kb crkeys.Keybase) (sdk.AccAddress, string) { @@ -288,7 +303,7 @@ func InitializeTestLCD( require.NoError(t, err) tests.WaitForNextHeightTM(tests.ExtractPortFromAddress(config.RPC.ListenAddress)) - lcd, err := startLCD(logger, listenAddr, cdc) + lcd, err := startLCD(logger, listenAddr, cdc, t) require.NoError(t, err) tests.WaitForLCDStart(port) @@ -347,8 +362,23 @@ func startTM( // startLCD starts the LCD. // // NOTE: This causes the thread to block. -func startLCD(logger log.Logger, listenAddr string, cdc *codec.Codec) (net.Listener, error) { - return tmrpc.StartHTTPServer(listenAddr, createHandler(cdc), logger, tmrpc.Config{}) +func startLCD(logger log.Logger, listenAddr string, cdc *codec.Codec, t *testing.T) (net.Listener, error) { + rs := NewRestServer(cdc) + rs.setKeybase(GetTestKeyBase(t)) + registerRoutes(rs) + return tmrpc.StartHTTPServer(listenAddr, rs.Mux, logger, tmrpc.Config{}) +} + +// NOTE: If making updates here also update cmd/gaia/cmd/gaiacli/main.go +func registerRoutes(rs *RestServer) { + keys.RegisterRoutes(rs.Mux, rs.CliCtx.Indent) + rpc.RegisterRoutes(rs.CliCtx, rs.Mux) + tx.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc) + authRest.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, "acc") + bankRest.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, rs.KeyBase) + stakeRest.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, rs.KeyBase) + slashingRest.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, rs.KeyBase) + govRest.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc) } // Request makes a test LCD test request. It returns a response object and a diff --git a/cmd/gaia/cmd/gaiacli/main.go b/cmd/gaia/cmd/gaiacli/main.go index de99b0fc8..3dd936f86 100644 --- a/cmd/gaia/cmd/gaiacli/main.go +++ b/cmd/gaia/cmd/gaiacli/main.go @@ -1,9 +1,11 @@ package main import ( + "net/http" "os" "path" + "github.com/rakyll/statik/fs" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -13,9 +15,15 @@ import ( "github.com/cosmos/cosmos-sdk/client/keys" "github.com/cosmos/cosmos-sdk/client/lcd" "github.com/cosmos/cosmos-sdk/client/rpc" + "github.com/cosmos/cosmos-sdk/client/tx" "github.com/cosmos/cosmos-sdk/cmd/gaia/app" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/version" + auth "github.com/cosmos/cosmos-sdk/x/auth/client/rest" + bank "github.com/cosmos/cosmos-sdk/x/bank/client/rest" + gov "github.com/cosmos/cosmos-sdk/x/gov/client/rest" + slashing "github.com/cosmos/cosmos-sdk/x/slashing/client/rest" + stake "github.com/cosmos/cosmos-sdk/x/stake/client/rest" _ "github.com/cosmos/cosmos-sdk/client/lcd/statik" ) @@ -37,15 +45,25 @@ var ( ) func main() { + // Configure cobra to sort commands cobra.EnableCommandSorting = false + + // Instantiate the codec for the command line application cdc := app.MakeCodec() + // Read in the configuration file for the sdk config := sdk.GetConfig() config.SetBech32PrefixForAccount(sdk.Bech32PrefixAccAddr, sdk.Bech32PrefixAccPub) config.SetBech32PrefixForValidator(sdk.Bech32PrefixValAddr, sdk.Bech32PrefixValPub) config.SetBech32PrefixForConsensusNode(sdk.Bech32PrefixConsAddr, sdk.Bech32PrefixConsPub) config.Seal() + // Create a new RestServer instance to serve the light client routes + rs := lcd.NewRestServer(cdc) + + // registerRoutes registers the routes on the rest server + registerRoutes(rs) + // TODO: setup keybase, viper object, etc. to be passed into // the below functions and eliminate global vars, like we do // with the cdc @@ -58,7 +76,7 @@ func main() { queryCmd(cdc), txCmd(cdc), client.LineBreak, - lcd.ServeCommand(cdc), + rs.ServeCommand(), client.LineBreak, keys.Commands(), client.LineBreak, @@ -79,6 +97,30 @@ func main() { } } +// registerRoutes registers the routes from the different modules for the LCD. +// NOTE: details on the routes added for each module are in the module documentation +// NOTE: If making updates here you also need to update the test helper in client/lcd/test_helper.go +func registerRoutes(rs *lcd.RestServer) { + registerSwaggerUI(rs) + keys.RegisterRoutes(rs.Mux, rs.CliCtx.Indent) + rpc.RegisterRoutes(rs.CliCtx, rs.Mux) + tx.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc) + auth.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, "acc") + bank.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, rs.KeyBase) + stake.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, rs.KeyBase) + slashing.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, rs.KeyBase) + gov.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc) +} + +func registerSwaggerUI(rs *lcd.RestServer) { + statikFS, err := fs.New() + if err != nil { + panic(err) + } + staticServer := http.FileServer(statikFS) + rs.Mux.PathPrefix("/swagger-ui/").Handler(http.StripPrefix("/swagger-ui/", staticServer)) +} + func initConfig(cmd *cobra.Command) error { home, err := cmd.PersistentFlags().GetString(cli.HomeFlag) if err != nil { diff --git a/docs/examples/basecoin/cmd/basecli/main.go b/docs/examples/basecoin/cmd/basecli/main.go index 3a16c8a97..ad32bf516 100644 --- a/docs/examples/basecoin/cmd/basecli/main.go +++ b/docs/examples/basecoin/cmd/basecli/main.go @@ -9,12 +9,17 @@ import ( "github.com/cosmos/cosmos-sdk/client/tx" "github.com/cosmos/cosmos-sdk/docs/examples/basecoin/app" "github.com/cosmos/cosmos-sdk/docs/examples/basecoin/types" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/version" authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" + auth "github.com/cosmos/cosmos-sdk/x/auth/client/rest" bankcmd "github.com/cosmos/cosmos-sdk/x/bank/client/cli" + bank "github.com/cosmos/cosmos-sdk/x/bank/client/rest" ibccmd "github.com/cosmos/cosmos-sdk/x/ibc/client/cli" slashingcmd "github.com/cosmos/cosmos-sdk/x/slashing/client/cli" + slashing "github.com/cosmos/cosmos-sdk/x/slashing/client/rest" stakecmd "github.com/cosmos/cosmos-sdk/x/stake/client/cli" + stake "github.com/cosmos/cosmos-sdk/x/stake/client/rest" "github.com/spf13/cobra" "github.com/tendermint/tendermint/libs/cli" ) @@ -34,6 +39,17 @@ func main() { // get the codec cdc := app.MakeCodec() + // Setup certain SDK config + config := sdk.GetConfig() + config.SetBech32PrefixForAccount("baseacc", "basepub") + config.SetBech32PrefixForValidator("baseval", "basevalpub") + config.SetBech32PrefixForConsensusNode("basecons", "baseconspub") + config.Seal() + + rs := lcd.NewRestServer(cdc) + + registerRoutes(rs) + // TODO: Setup keybase, viper object, etc. to be passed into // the below functions and eliminate global vars, like we do // with the cdc. @@ -83,7 +99,7 @@ func main() { // add proxy, version and key info rootCmd.AddCommand( client.LineBreak, - lcd.ServeCommand(cdc), + rs.ServeCommand(), keys.Commands(), client.LineBreak, version.VersionCmd, @@ -97,3 +113,13 @@ func main() { panic(err) } } + +func registerRoutes(rs *lcd.RestServer) { + keys.RegisterRoutes(rs.Mux, rs.CliCtx.Indent) + rpc.RegisterRoutes(rs.CliCtx, rs.Mux) + tx.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc) + auth.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, "acc") + bank.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, rs.KeyBase) + stake.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, rs.KeyBase) + slashing.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, rs.KeyBase) +} diff --git a/docs/examples/democoin/cmd/democli/main.go b/docs/examples/democoin/cmd/democli/main.go index adb6169c9..d101da2d9 100644 --- a/docs/examples/democoin/cmd/democli/main.go +++ b/docs/examples/democoin/cmd/democli/main.go @@ -13,8 +13,9 @@ import ( "github.com/cosmos/cosmos-sdk/version" authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" + auth "github.com/cosmos/cosmos-sdk/x/auth/client/rest" bankcmd "github.com/cosmos/cosmos-sdk/x/bank/client/cli" - ibccmd "github.com/cosmos/cosmos-sdk/x/ibc/client/cli" + bank "github.com/cosmos/cosmos-sdk/x/bank/client/rest" "github.com/cosmos/cosmos-sdk/docs/examples/democoin/app" "github.com/cosmos/cosmos-sdk/docs/examples/democoin/types" @@ -47,6 +48,10 @@ func main() { config.SetBech32PrefixForConsensusNode("democons", "democonspub") config.Seal() + rs := lcd.NewRestServer(cdc) + + registerRoutes(rs) + // TODO: setup keybase, viper object, etc. to be passed into // the below functions and eliminate global vars, like we do // with the cdc @@ -74,11 +79,6 @@ func main() { )...) rootCmd.AddCommand( client.PostCommands( - ibccmd.IBCTransferCmd(cdc), - )...) - rootCmd.AddCommand( - client.PostCommands( - ibccmd.IBCRelayCmd(cdc), simplestakingcmd.BondTxCmd(cdc), )...) rootCmd.AddCommand( @@ -96,7 +96,7 @@ func main() { // add proxy, version and key info rootCmd.AddCommand( client.LineBreak, - lcd.ServeCommand(cdc), + rs.ServeCommand(), keys.Commands(), client.LineBreak, version.VersionCmd, @@ -110,3 +110,11 @@ func main() { panic(err) } } + +func registerRoutes(rs *lcd.RestServer) { + keys.RegisterRoutes(rs.Mux, rs.CliCtx.Indent) + rpc.RegisterRoutes(rs.CliCtx, rs.Mux) + tx.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc) + auth.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, "acc") + bank.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, rs.KeyBase) +} From fd968f7d8fb748bd5e2046dec358b6a85a8c73bf Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Mon, 19 Nov 2018 16:42:53 +0100 Subject: [PATCH 4/9] R4R: Remove unused bank.MsgIssue (and prevent possible panic) (#2855) * Remove all bank.MsgIssue code --- PENDING.md | 1 + x/bank/codec.go | 1 - x/bank/handler.go | 7 ----- x/bank/msgs.go | 59 ----------------------------------- x/bank/msgs_test.go | 45 -------------------------- x/ibc/ibc_test.go | 1 - x/stake/keeper/test_common.go | 1 - 7 files changed, 1 insertion(+), 114 deletions(-) diff --git a/PENDING.md b/PENDING.md index a8f278ebb..4720ec774 100644 --- a/PENDING.md +++ b/PENDING.md @@ -80,6 +80,7 @@ BUG FIXES * SDK - \#2733 [x/gov, x/mock/simulation] Fix governance simulation, update x/gov import/export + - \#2854 [x/bank] Remove unused bank.MsgIssue, prevent possible panic * Tendermint * [\#2797](https://github.com/tendermint/tendermint/pull/2797) AddressBook requires addresses to have IDs; Do not crap out immediately after sending pex addrs in seed mode diff --git a/x/bank/codec.go b/x/bank/codec.go index bcc2cbdda..2195e4853 100644 --- a/x/bank/codec.go +++ b/x/bank/codec.go @@ -7,7 +7,6 @@ import ( // Register concrete types on codec codec func RegisterCodec(cdc *codec.Codec) { cdc.RegisterConcrete(MsgSend{}, "cosmos-sdk/Send", nil) - cdc.RegisterConcrete(MsgIssue{}, "cosmos-sdk/Issue", nil) } var msgCdc = codec.New() diff --git a/x/bank/handler.go b/x/bank/handler.go index e02043d7b..ea3ee4398 100644 --- a/x/bank/handler.go +++ b/x/bank/handler.go @@ -10,8 +10,6 @@ func NewHandler(k Keeper) sdk.Handler { switch msg := msg.(type) { case MsgSend: return handleMsgSend(ctx, k, msg) - case MsgIssue: - return handleMsgIssue(ctx, k, msg) default: errMsg := "Unrecognized bank Msg type: %s" + msg.Type() return sdk.ErrUnknownRequest(errMsg).Result() @@ -32,8 +30,3 @@ func handleMsgSend(ctx sdk.Context, k Keeper, msg MsgSend) sdk.Result { Tags: tags, } } - -// Handle MsgIssue. -func handleMsgIssue(ctx sdk.Context, k Keeper, msg MsgIssue) sdk.Result { - panic("not implemented yet") -} diff --git a/x/bank/msgs.go b/x/bank/msgs.go index a1c346a88..1af7acfe7 100644 --- a/x/bank/msgs.go +++ b/x/bank/msgs.go @@ -86,65 +86,6 @@ func (msg MsgSend) GetSigners() []sdk.AccAddress { return addrs } -//---------------------------------------- -// MsgIssue - -// MsgIssue - high level transaction of the coin module -type MsgIssue struct { - Banker sdk.AccAddress `json:"banker"` - Outputs []Output `json:"outputs"` -} - -var _ sdk.Msg = MsgIssue{} - -// NewMsgIssue - construct arbitrary multi-in, multi-out send msg. -func NewMsgIssue(banker sdk.AccAddress, out []Output) MsgIssue { - return MsgIssue{Banker: banker, Outputs: out} -} - -// Implements Msg. -// nolint -func (msg MsgIssue) Route() string { return "bank" } // TODO: "bank/issue" -func (msg MsgIssue) Type() string { return "issue" } - -// Implements Msg. -func (msg MsgIssue) ValidateBasic() sdk.Error { - // XXX - if len(msg.Outputs) == 0 { - return ErrNoOutputs(DefaultCodespace).TraceSDK("") - } - for _, out := range msg.Outputs { - if err := out.ValidateBasic(); err != nil { - return err.TraceSDK("") - } - } - return nil -} - -// Implements Msg. -func (msg MsgIssue) GetSignBytes() []byte { - var outputs []json.RawMessage - for _, output := range msg.Outputs { - outputs = append(outputs, output.GetSignBytes()) - } - b, err := msgCdc.MarshalJSON(struct { - Banker sdk.AccAddress `json:"banker"` - Outputs []json.RawMessage `json:"outputs"` - }{ - Banker: msg.Banker, - Outputs: outputs, - }) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) -} - -// Implements Msg. -func (msg MsgIssue) GetSigners() []sdk.AccAddress { - return []sdk.AccAddress{msg.Banker} -} - //---------------------------------------- // Input diff --git a/x/bank/msgs_test.go b/x/bank/msgs_test.go index 157cc9b5b..14e9a4c70 100644 --- a/x/bank/msgs_test.go +++ b/x/bank/msgs_test.go @@ -223,48 +223,3 @@ func TestMsgSendSigners(t *testing.T) { require.Equal(t, signers, tx.Signers()) } */ - -// ---------------------------------------- -// MsgIssue Tests - -func TestNewMsgIssue(t *testing.T) { - // TODO -} - -func TestMsgIssueRoute(t *testing.T) { - // Construct an MsgIssue - addr := sdk.AccAddress([]byte("loan-from-bank")) - coins := sdk.Coins{sdk.NewInt64Coin("atom", 10)} - var msg = MsgIssue{ - Banker: sdk.AccAddress([]byte("input")), - Outputs: []Output{NewOutput(addr, coins)}, - } - - // TODO some failures for bad result - require.Equal(t, msg.Route(), "bank") -} - -func TestMsgIssueValidation(t *testing.T) { - // TODO -} - -func TestMsgIssueGetSignBytes(t *testing.T) { - addr := sdk.AccAddress([]byte("loan-from-bank")) - coins := sdk.Coins{sdk.NewInt64Coin("atom", 10)} - var msg = MsgIssue{ - Banker: sdk.AccAddress([]byte("input")), - Outputs: []Output{NewOutput(addr, coins)}, - } - res := msg.GetSignBytes() - - expected := `{"banker":"cosmos1d9h8qat57ljhcm","outputs":[{"address":"cosmos1d3hkzm3dveex7mfdvfsku6cjngpcj","coins":[{"amount":"10","denom":"atom"}]}]}` - require.Equal(t, expected, string(res)) -} - -func TestMsgIssueGetSigners(t *testing.T) { - var msg = MsgIssue{ - Banker: sdk.AccAddress([]byte("onlyone")), - } - res := msg.GetSigners() - require.Equal(t, fmt.Sprintf("%v", res), "[6F6E6C796F6E65]") -} diff --git a/x/ibc/ibc_test.go b/x/ibc/ibc_test.go index 6cd89fded..94c3c7a2e 100644 --- a/x/ibc/ibc_test.go +++ b/x/ibc/ibc_test.go @@ -44,7 +44,6 @@ func makeCodec() *codec.Codec { // Register Msgs cdc.RegisterInterface((*sdk.Msg)(nil), nil) cdc.RegisterConcrete(bank.MsgSend{}, "test/ibc/Send", nil) - cdc.RegisterConcrete(bank.MsgIssue{}, "test/ibc/Issue", nil) cdc.RegisterConcrete(IBCTransferMsg{}, "test/ibc/IBCTransferMsg", nil) cdc.RegisterConcrete(IBCReceiveMsg{}, "test/ibc/IBCReceiveMsg", nil) diff --git a/x/stake/keeper/test_common.go b/x/stake/keeper/test_common.go index af7e688f3..65445cbb1 100644 --- a/x/stake/keeper/test_common.go +++ b/x/stake/keeper/test_common.go @@ -60,7 +60,6 @@ func MakeTestCodec() *codec.Codec { // Register Msgs cdc.RegisterInterface((*sdk.Msg)(nil), nil) cdc.RegisterConcrete(bank.MsgSend{}, "test/stake/Send", nil) - cdc.RegisterConcrete(bank.MsgIssue{}, "test/stake/Issue", nil) cdc.RegisterConcrete(types.MsgCreateValidator{}, "test/stake/CreateValidator", nil) cdc.RegisterConcrete(types.MsgEditValidator{}, "test/stake/EditValidator", nil) cdc.RegisterConcrete(types.MsgBeginUnbonding{}, "test/stake/BeginUnbonding", nil) From f525717054b9f3d09212ba7523ac98a2489d95ad Mon Sep 17 00:00:00 2001 From: Jack Zampolin Date: Mon, 19 Nov 2018 09:02:34 -0800 Subject: [PATCH 5/9] Standardize CLI Exports from Modules (#2840) * Move query and tx commands to modules * Move GetAccountDecoder to prevent import cycle and replace calls to it with one call in WithAccountDecoder * Add moduleClients interface and implement in all applicable modules * Use module clients in cli initialization --- PENDING.md | 1 + client/context/context.go | 16 +- client/lcd/root.go | 3 + cmd/gaia/cmd/gaiacli/main.go | 96 +++++- cmd/gaia/cmd/gaiacli/query.go | 83 ----- cmd/gaia/cmd/gaiacli/tx.go | 83 ----- cmd/gaia/init/gentx.go | 9 +- docs/examples/basecoin/cmd/basecli/main.go | 42 ++- docs/examples/democoin/cmd/democli/main.go | 6 +- .../examples/democoin/x/cool/client/cli/tx.go | 5 +- docs/examples/democoin/x/pow/client/cli/tx.go | 3 +- .../x/simplestake/client/cli/commands.go | 3 +- types/module_clients.go | 11 + x/auth/client/cli/account.go | 30 +- x/auth/client/cli/sign.go | 15 +- x/auth/client/rest/query.go | 5 +- x/bank/client/cli/broadcast.go | 3 +- x/bank/client/cli/sendtx.go | 10 +- x/distribution/client/cli/tx.go | 22 +- x/distribution/client/module_client.go | 38 ++ x/gov/client/cli/query.go | 324 ++++++++++++++++++ x/gov/client/cli/tx.go | 324 +----------------- x/gov/client/module_client.go | 55 +++ x/gov/client/rest/rest.go | 8 +- x/gov/client/{ => utils}/utils.go | 2 +- x/ibc/client/cli/ibctx.go | 3 +- x/ibc/client/cli/relay.go | 3 +- x/slashing/client/cli/tx.go | 3 +- x/slashing/client/module_client.go | 47 +++ x/stake/client/cli/query.go | 12 +- x/stake/client/cli/tx.go | 11 +- x/stake/client/cli/utils.go | 3 +- x/stake/client/module_client.go | 61 ++++ 33 files changed, 739 insertions(+), 601 deletions(-) delete mode 100644 cmd/gaia/cmd/gaiacli/query.go delete mode 100644 cmd/gaia/cmd/gaiacli/tx.go create mode 100644 types/module_clients.go create mode 100644 x/distribution/client/module_client.go create mode 100644 x/gov/client/cli/query.go create mode 100644 x/gov/client/module_client.go rename x/gov/client/{ => utils}/utils.go (98%) create mode 100644 x/slashing/client/module_client.go create mode 100644 x/stake/client/module_client.go diff --git a/PENDING.md b/PENDING.md index 4720ec774..1c17731e2 100644 --- a/PENDING.md +++ b/PENDING.md @@ -31,6 +31,7 @@ FEATURES * [gov][cli] [\#2479](https://github.com/cosmos/cosmos-sdk/issues/2479) Added governance parameter query commands. * [stake][cli] [\#2027] Add CLI query command for getting all delegations to a specific validator. + * [\#2840](https://github.com/cosmos/cosmos-sdk/pull/2840) Standardize CLI exports from modules * Gaia * [app] \#2791 Support export at a specific height, with `gaiad export --height=HEIGHT`. diff --git a/client/context/context.go b/client/context/context.go index 9108b3d0b..4b4407368 100644 --- a/client/context/context.go +++ b/client/context/context.go @@ -175,10 +175,22 @@ func (ctx CLIContext) WithCodec(cdc *codec.Codec) CLIContext { return ctx } +// GetAccountDecoder gets the account decoder for auth.DefaultAccount. +func GetAccountDecoder(cdc *codec.Codec) auth.AccountDecoder { + return func(accBytes []byte) (acct auth.Account, err error) { + err = cdc.UnmarshalBinaryBare(accBytes, &acct) + if err != nil { + panic(err) + } + + return acct, err + } +} + // WithAccountDecoder returns a copy of the context with an updated account // decoder. -func (ctx CLIContext) WithAccountDecoder(decoder auth.AccountDecoder) CLIContext { - ctx.AccDecoder = decoder +func (ctx CLIContext) WithAccountDecoder(cdc *codec.Codec) CLIContext { + ctx.AccDecoder = GetAccountDecoder(cdc) return ctx } diff --git a/client/lcd/root.go b/client/lcd/root.go index 3d6181646..7081b76c0 100644 --- a/client/lcd/root.go +++ b/client/lcd/root.go @@ -19,6 +19,9 @@ import ( "github.com/spf13/viper" "github.com/tendermint/tendermint/libs/log" tmserver "github.com/tendermint/tendermint/rpc/lib/server" + + // Import statik for light client stuff + _ "github.com/cosmos/cosmos-sdk/client/lcd/statik" ) // RestServer represents the Light Client Rest server diff --git a/cmd/gaia/cmd/gaiacli/main.go b/cmd/gaia/cmd/gaiacli/main.go index 3dd936f86..05c7bc4ec 100644 --- a/cmd/gaia/cmd/gaiacli/main.go +++ b/cmd/gaia/cmd/gaiacli/main.go @@ -1,6 +1,7 @@ package main import ( + "fmt" "net/http" "os" "path" @@ -9,6 +10,7 @@ import ( "github.com/spf13/cobra" "github.com/spf13/viper" + amino "github.com/tendermint/go-amino" "github.com/tendermint/tendermint/libs/cli" "github.com/cosmos/cosmos-sdk/client" @@ -19,29 +21,29 @@ import ( "github.com/cosmos/cosmos-sdk/cmd/gaia/app" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/version" + auth "github.com/cosmos/cosmos-sdk/x/auth/client/rest" bank "github.com/cosmos/cosmos-sdk/x/bank/client/rest" gov "github.com/cosmos/cosmos-sdk/x/gov/client/rest" slashing "github.com/cosmos/cosmos-sdk/x/slashing/client/rest" stake "github.com/cosmos/cosmos-sdk/x/stake/client/rest" + authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" + bankcmd "github.com/cosmos/cosmos-sdk/x/bank/client/cli" + distClient "github.com/cosmos/cosmos-sdk/x/distribution/client" + govClient "github.com/cosmos/cosmos-sdk/x/gov/client" + slashingClient "github.com/cosmos/cosmos-sdk/x/slashing/client" + stakeClient "github.com/cosmos/cosmos-sdk/x/stake/client" + _ "github.com/cosmos/cosmos-sdk/client/lcd/statik" ) const ( - storeAcc = "acc" - storeGov = "gov" - storeSlashing = "slashing" - storeStake = "stake" - queryRouteStake = "stake" -) - -// rootCmd is the entry point for this binary -var ( - rootCmd = &cobra.Command{ - Use: "gaiacli", - Short: "Command line interface for interacting with gaiad", - } + storeAcc = "acc" + storeGov = "gov" + storeSlashing = "slashing" + storeStake = "stake" + storeDist = "distr" ) func main() { @@ -68,13 +70,27 @@ func main() { // the below functions and eliminate global vars, like we do // with the cdc + // Module clients hold cli commnads (tx,query) and lcd routes + // TODO: Make the lcd command take a list of ModuleClient + mc := []sdk.ModuleClients{ + govClient.NewModuleClient(storeGov, cdc), + distClient.NewModuleClient(storeDist, cdc), + stakeClient.NewModuleClient(storeStake, cdc), + slashingClient.NewModuleClient(storeSlashing, cdc), + } + + rootCmd := &cobra.Command{ + Use: "gaiacli", + Short: "Command line interface for interacting with gaiad", + } + // Construct Root Command rootCmd.AddCommand( rpc.InitClientCommand(), rpc.StatusCommand(), client.ConfigCmd(), - queryCmd(cdc), - txCmd(cdc), + queryCmd(cdc, mc), + txCmd(cdc, mc), client.LineBreak, rs.ServeCommand(), client.LineBreak, @@ -92,11 +108,55 @@ func main() { err = executor.Execute() if err != nil { - // handle with #870 - panic(err) + fmt.Printf("Failed executing CLI command: %s, exiting...\n", err) + os.Exit(1) } } +func queryCmd(cdc *amino.Codec, mc []sdk.ModuleClients) *cobra.Command { + queryCmd := &cobra.Command{ + Use: "query", + Aliases: []string{"q"}, + Short: "Querying subcommands", + } + + queryCmd.AddCommand( + rpc.ValidatorCommand(), + rpc.BlockCommand(), + tx.SearchTxCmd(cdc), + tx.QueryTxCmd(cdc), + client.LineBreak, + authcmd.GetAccountCmd(storeAcc, cdc), + ) + + for _, m := range mc { + queryCmd.AddCommand(m.GetQueryCmd()) + } + + return queryCmd +} + +func txCmd(cdc *amino.Codec, mc []sdk.ModuleClients) *cobra.Command { + txCmd := &cobra.Command{ + Use: "tx", + Short: "Transactions subcommands", + } + + txCmd.AddCommand( + bankcmd.SendTxCmd(cdc), + client.LineBreak, + authcmd.GetSignCommand(cdc), + bankcmd.GetBroadcastCommand(cdc), + client.LineBreak, + ) + + for _, m := range mc { + txCmd.AddCommand(m.GetTxCmd()) + } + + return txCmd +} + // registerRoutes registers the routes from the different modules for the LCD. // NOTE: details on the routes added for each module are in the module documentation // NOTE: If making updates here you also need to update the test helper in client/lcd/test_helper.go @@ -105,7 +165,7 @@ func registerRoutes(rs *lcd.RestServer) { keys.RegisterRoutes(rs.Mux, rs.CliCtx.Indent) rpc.RegisterRoutes(rs.CliCtx, rs.Mux) tx.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc) - auth.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, "acc") + auth.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, storeAcc) bank.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, rs.KeyBase) stake.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, rs.KeyBase) slashing.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, rs.KeyBase) diff --git a/cmd/gaia/cmd/gaiacli/query.go b/cmd/gaia/cmd/gaiacli/query.go deleted file mode 100644 index 3a806d36c..000000000 --- a/cmd/gaia/cmd/gaiacli/query.go +++ /dev/null @@ -1,83 +0,0 @@ -package main - -import ( - "github.com/cosmos/cosmos-sdk/client" - "github.com/cosmos/cosmos-sdk/client/rpc" - "github.com/cosmos/cosmos-sdk/client/tx" - "github.com/spf13/cobra" - - authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" - govcmd "github.com/cosmos/cosmos-sdk/x/gov/client/cli" - slashingcmd "github.com/cosmos/cosmos-sdk/x/slashing/client/cli" - stakecmd "github.com/cosmos/cosmos-sdk/x/stake/client/cli" - amino "github.com/tendermint/go-amino" -) - -func queryCmd(cdc *amino.Codec) *cobra.Command { - //Add query commands - queryCmd := &cobra.Command{ - Use: "query", - Aliases: []string{"q"}, - Short: "Querying subcommands", - } - - // Group staking queries under a subcommand - stakeQueryCmd := &cobra.Command{ - Use: "stake", - Short: "Querying commands for the staking module", - } - - stakeQueryCmd.AddCommand(client.GetCommands( - stakecmd.GetCmdQueryDelegation(storeStake, cdc), - stakecmd.GetCmdQueryDelegations(storeStake, cdc), - stakecmd.GetCmdQueryUnbondingDelegation(storeStake, cdc), - stakecmd.GetCmdQueryUnbondingDelegations(storeStake, cdc), - stakecmd.GetCmdQueryRedelegation(storeStake, cdc), - stakecmd.GetCmdQueryRedelegations(storeStake, cdc), - stakecmd.GetCmdQueryValidator(storeStake, cdc), - stakecmd.GetCmdQueryValidators(storeStake, cdc), - stakecmd.GetCmdQueryValidatorDelegations(storeStake, cdc), - stakecmd.GetCmdQueryValidatorUnbondingDelegations(queryRouteStake, cdc), - stakecmd.GetCmdQueryValidatorRedelegations(queryRouteStake, cdc), - stakecmd.GetCmdQueryParams(storeStake, cdc), - stakecmd.GetCmdQueryPool(storeStake, cdc))...) - - // Group gov queries under a subcommand - govQueryCmd := &cobra.Command{ - Use: "gov", - Short: "Querying commands for the governance module", - } - - govQueryCmd.AddCommand(client.GetCommands( - govcmd.GetCmdQueryProposal(storeGov, cdc), - govcmd.GetCmdQueryProposals(storeGov, cdc), - govcmd.GetCmdQueryVote(storeGov, cdc), - govcmd.GetCmdQueryVotes(storeGov, cdc), - govcmd.GetCmdQueryParams(storeGov, cdc), - govcmd.GetCmdQueryDeposit(storeGov, cdc), - govcmd.GetCmdQueryDeposits(storeGov, cdc))...) - - // Group slashing queries under a subcommand - slashingQueryCmd := &cobra.Command{ - Use: "slashing", - Short: "Querying commands for the slashing module", - } - - slashingQueryCmd.AddCommand(client.GetCommands( - slashingcmd.GetCmdQuerySigningInfo(storeSlashing, cdc))...) - - // Query commcmmand structure - queryCmd.AddCommand( - rpc.BlockCommand(), - rpc.ValidatorCommand(), - tx.SearchTxCmd(cdc), - tx.QueryTxCmd(cdc), - client.LineBreak, - client.GetCommands(authcmd.GetAccountCmd(storeAcc, cdc, authcmd.GetAccountDecoder(cdc)))[0], - stakeQueryCmd, - govQueryCmd, - slashingQueryCmd, - ) - - return queryCmd -} diff --git a/cmd/gaia/cmd/gaiacli/tx.go b/cmd/gaia/cmd/gaiacli/tx.go deleted file mode 100644 index fa0abc4ad..000000000 --- a/cmd/gaia/cmd/gaiacli/tx.go +++ /dev/null @@ -1,83 +0,0 @@ -package main - -import ( - "github.com/cosmos/cosmos-sdk/client" - "github.com/spf13/cobra" - - authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" - bankcmd "github.com/cosmos/cosmos-sdk/x/bank/client/cli" - distrcmd "github.com/cosmos/cosmos-sdk/x/distribution/client/cli" - govcmd "github.com/cosmos/cosmos-sdk/x/gov/client/cli" - slashingcmd "github.com/cosmos/cosmos-sdk/x/slashing/client/cli" - stakecmd "github.com/cosmos/cosmos-sdk/x/stake/client/cli" - amino "github.com/tendermint/go-amino" -) - -func txCmd(cdc *amino.Codec) *cobra.Command { - //Add transaction generation commands - txCmd := &cobra.Command{ - Use: "tx", - Short: "Transactions subcommands", - } - - stakeTxCmd := &cobra.Command{ - Use: "stake", - Short: "Staking transaction subcommands", - } - - stakeTxCmd.AddCommand(client.PostCommands( - stakecmd.GetCmdCreateValidator(cdc), - stakecmd.GetCmdEditValidator(cdc), - stakecmd.GetCmdDelegate(cdc), - stakecmd.GetCmdRedelegate(storeStake, cdc), - stakecmd.GetCmdUnbond(storeStake, cdc), - )...) - - distTxCmd := &cobra.Command{ - Use: "dist", - Short: "Distribution transactions subcommands", - } - - distTxCmd.AddCommand(client.PostCommands( - distrcmd.GetCmdWithdrawRewards(cdc), - distrcmd.GetCmdSetWithdrawAddr(cdc), - )...) - - govTxCmd := &cobra.Command{ - Use: "gov", - Short: "Governance transactions subcommands", - } - - govTxCmd.AddCommand(client.PostCommands( - govcmd.GetCmdDeposit(cdc), - govcmd.GetCmdVote(cdc), - govcmd.GetCmdSubmitProposal(cdc), - )...) - - slashingTxCmd := &cobra.Command{ - Use: "slashing", - Short: "Slashing transactions subcommands", - } - - slashingTxCmd.AddCommand(client.PostCommands( - slashingcmd.GetCmdUnjail(cdc), - )...) - - txCmd.AddCommand( - //Add auth and bank commands - client.PostCommands( - bankcmd.SendTxCmd(cdc), - bankcmd.GetBroadcastCommand(cdc), - authcmd.GetSignCommand(cdc, authcmd.GetAccountDecoder(cdc)), - )...) - - txCmd.AddCommand( - client.LineBreak, - stakeTxCmd, - distTxCmd, - govTxCmd, - slashingTxCmd, - ) - - return txCmd -} diff --git a/cmd/gaia/init/gentx.go b/cmd/gaia/init/gentx.go index 6d66f2e30..096097e18 100644 --- a/cmd/gaia/init/gentx.go +++ b/cmd/gaia/init/gentx.go @@ -2,6 +2,10 @@ package init import ( "fmt" + "io/ioutil" + "os" + "path/filepath" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/keys" "github.com/cosmos/cosmos-sdk/cmd/gaia/app" @@ -17,9 +21,6 @@ import ( "github.com/tendermint/tendermint/crypto" tmcli "github.com/tendermint/tendermint/libs/cli" "github.com/tendermint/tendermint/libs/common" - "io/ioutil" - "os" - "path/filepath" ) const ( @@ -94,7 +95,7 @@ following delegation and commission default parameters: w.Close() prepareFlagsForTxSign() - signCmd := authcmd.GetSignCommand(cdc, authcmd.GetAccountDecoder(cdc)) + signCmd := authcmd.GetSignCommand(cdc) if w, err = prepareOutputFile(config.RootDir, nodeID); err != nil { return err } diff --git a/docs/examples/basecoin/cmd/basecli/main.go b/docs/examples/basecoin/cmd/basecli/main.go index ad32bf516..cb0eeba5b 100644 --- a/docs/examples/basecoin/cmd/basecli/main.go +++ b/docs/examples/basecoin/cmd/basecli/main.go @@ -8,7 +8,7 @@ import ( "github.com/cosmos/cosmos-sdk/client/rpc" "github.com/cosmos/cosmos-sdk/client/tx" "github.com/cosmos/cosmos-sdk/docs/examples/basecoin/app" - "github.com/cosmos/cosmos-sdk/docs/examples/basecoin/types" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/version" authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" @@ -24,6 +24,12 @@ import ( "github.com/tendermint/tendermint/libs/cli" ) +const ( + storeAcc = "acc" + storeSlashing = "slashing" + storeStake = "stake" +) + // rootCmd is the entry point for this binary var ( rootCmd = &cobra.Command{ @@ -67,20 +73,20 @@ func main() { // add query/post commands (custom to binary) rootCmd.AddCommand( client.GetCommands( - stakecmd.GetCmdQueryValidator("stake", cdc), - stakecmd.GetCmdQueryValidators("stake", cdc), - stakecmd.GetCmdQueryValidatorUnbondingDelegations("stake", cdc), - stakecmd.GetCmdQueryValidatorRedelegations("stake", cdc), - stakecmd.GetCmdQueryDelegation("stake", cdc), - stakecmd.GetCmdQueryDelegations("stake", cdc), - stakecmd.GetCmdQueryPool("stake", cdc), - stakecmd.GetCmdQueryParams("stake", cdc), - stakecmd.GetCmdQueryUnbondingDelegation("stake", cdc), - stakecmd.GetCmdQueryUnbondingDelegations("stake", cdc), - stakecmd.GetCmdQueryRedelegation("stake", cdc), - stakecmd.GetCmdQueryRedelegations("stake", cdc), - slashingcmd.GetCmdQuerySigningInfo("slashing", cdc), - authcmd.GetAccountCmd("acc", cdc, types.GetAccountDecoder(cdc)), + stakecmd.GetCmdQueryValidator(storeStake, cdc), + stakecmd.GetCmdQueryValidators(storeStake, cdc), + stakecmd.GetCmdQueryValidatorUnbondingDelegations(storeStake, cdc), + stakecmd.GetCmdQueryValidatorRedelegations(storeStake, cdc), + stakecmd.GetCmdQueryDelegation(storeStake, cdc), + stakecmd.GetCmdQueryDelegations(storeStake, cdc), + stakecmd.GetCmdQueryPool(storeStake, cdc), + stakecmd.GetCmdQueryParams(storeStake, cdc), + stakecmd.GetCmdQueryUnbondingDelegation(storeStake, cdc), + stakecmd.GetCmdQueryUnbondingDelegations(storeStake, cdc), + stakecmd.GetCmdQueryRedelegation(storeStake, cdc), + stakecmd.GetCmdQueryRedelegations(storeStake, cdc), + slashingcmd.GetCmdQuerySigningInfo(storeSlashing, cdc), + authcmd.GetAccountCmd(storeAcc, cdc), )...) rootCmd.AddCommand( @@ -91,8 +97,8 @@ func main() { stakecmd.GetCmdCreateValidator(cdc), stakecmd.GetCmdEditValidator(cdc), stakecmd.GetCmdDelegate(cdc), - stakecmd.GetCmdUnbond("stake", cdc), - stakecmd.GetCmdRedelegate("stake", cdc), + stakecmd.GetCmdUnbond(storeStake, cdc), + stakecmd.GetCmdRedelegate(storeStake, cdc), slashingcmd.GetCmdUnjail(cdc), )...) @@ -118,7 +124,7 @@ func registerRoutes(rs *lcd.RestServer) { keys.RegisterRoutes(rs.Mux, rs.CliCtx.Indent) rpc.RegisterRoutes(rs.CliCtx, rs.Mux) tx.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc) - auth.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, "acc") + auth.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, storeAcc) bank.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, rs.KeyBase) stake.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, rs.KeyBase) slashing.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, rs.KeyBase) diff --git a/docs/examples/democoin/cmd/democli/main.go b/docs/examples/democoin/cmd/democli/main.go index d101da2d9..0c37d9bd0 100644 --- a/docs/examples/democoin/cmd/democli/main.go +++ b/docs/examples/democoin/cmd/democli/main.go @@ -18,7 +18,6 @@ import ( bank "github.com/cosmos/cosmos-sdk/x/bank/client/rest" "github.com/cosmos/cosmos-sdk/docs/examples/democoin/app" - "github.com/cosmos/cosmos-sdk/docs/examples/democoin/types" coolcmd "github.com/cosmos/cosmos-sdk/docs/examples/democoin/x/cool/client/cli" powcmd "github.com/cosmos/cosmos-sdk/docs/examples/democoin/x/pow/client/cli" simplestakingcmd "github.com/cosmos/cosmos-sdk/docs/examples/democoin/x/simplestake/client/cli" @@ -32,6 +31,7 @@ var ( Use: "democli", Short: "Democoin light-client", } + storeAcc = "acc" ) func main() { @@ -71,7 +71,7 @@ func main() { // start with commands common to basecoin rootCmd.AddCommand( client.GetCommands( - authcmd.GetAccountCmd("acc", cdc, types.GetAccountDecoder(cdc)), + authcmd.GetAccountCmd(storeAcc, cdc), )...) rootCmd.AddCommand( client.PostCommands( @@ -115,6 +115,6 @@ func registerRoutes(rs *lcd.RestServer) { keys.RegisterRoutes(rs.Mux, rs.CliCtx.Indent) rpc.RegisterRoutes(rs.CliCtx, rs.Mux) tx.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc) - auth.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, "acc") + auth.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, storeAcc) bank.RegisterRoutes(rs.CliCtx, rs.Mux, rs.Cdc, rs.KeyBase) } diff --git a/docs/examples/democoin/x/cool/client/cli/tx.go b/docs/examples/democoin/x/cool/client/cli/tx.go index a21685a24..aedbd3711 100644 --- a/docs/examples/democoin/x/cool/client/cli/tx.go +++ b/docs/examples/democoin/x/cool/client/cli/tx.go @@ -8,7 +8,6 @@ import ( "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/docs/examples/democoin/x/cool" sdk "github.com/cosmos/cosmos-sdk/types" - authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" authtxb "github.com/cosmos/cosmos-sdk/x/auth/client/txbuilder" ) @@ -22,7 +21,7 @@ func QuizTxCmd(cdc *codec.Codec) *cobra.Command { txBldr := authtxb.NewTxBuilderFromCLI().WithCodec(cdc) cliCtx := context.NewCLIContext(). WithCodec(cdc). - WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) + WithAccountDecoder(cdc) from, err := cliCtx.GetFromAddress() if err != nil { @@ -46,7 +45,7 @@ func SetTrendTxCmd(cdc *codec.Codec) *cobra.Command { txBldr := authtxb.NewTxBuilderFromCLI().WithCodec(cdc) cliCtx := context.NewCLIContext(). WithCodec(cdc). - WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) + WithAccountDecoder(cdc) from, err := cliCtx.GetFromAddress() if err != nil { diff --git a/docs/examples/democoin/x/pow/client/cli/tx.go b/docs/examples/democoin/x/pow/client/cli/tx.go index 548aa9910..a6827c3ab 100644 --- a/docs/examples/democoin/x/pow/client/cli/tx.go +++ b/docs/examples/democoin/x/pow/client/cli/tx.go @@ -8,7 +8,6 @@ import ( "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/docs/examples/democoin/x/pow" sdk "github.com/cosmos/cosmos-sdk/types" - authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" authtxb "github.com/cosmos/cosmos-sdk/x/auth/client/txbuilder" "github.com/spf13/cobra" @@ -24,7 +23,7 @@ func MineCmd(cdc *codec.Codec) *cobra.Command { txBldr := authtxb.NewTxBuilderFromCLI().WithCodec(cdc) cliCtx := context.NewCLIContext(). WithCodec(cdc). - WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) + WithAccountDecoder(cdc) from, err := cliCtx.GetFromAddress() if err != nil { diff --git a/docs/examples/democoin/x/simplestake/client/cli/commands.go b/docs/examples/democoin/x/simplestake/client/cli/commands.go index 3fe9c20c4..582ea33e3 100644 --- a/docs/examples/democoin/x/simplestake/client/cli/commands.go +++ b/docs/examples/democoin/x/simplestake/client/cli/commands.go @@ -9,7 +9,6 @@ import ( "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/docs/examples/democoin/x/simplestake" sdk "github.com/cosmos/cosmos-sdk/types" - authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" authtxb "github.com/cosmos/cosmos-sdk/x/auth/client/txbuilder" "github.com/spf13/cobra" @@ -32,7 +31,7 @@ func BondTxCmd(cdc *codec.Codec) *cobra.Command { txBldr := authtxb.NewTxBuilderFromCLI().WithCodec(cdc) cliCtx := context.NewCLIContext(). WithCodec(cdc). - WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) + WithAccountDecoder(cdc) from, err := cliCtx.GetFromAddress() if err != nil { diff --git a/types/module_clients.go b/types/module_clients.go new file mode 100644 index 000000000..3b3a9d9a5 --- /dev/null +++ b/types/module_clients.go @@ -0,0 +1,11 @@ +package types + +import ( + "github.com/spf13/cobra" +) + +// ModuleClients helps modules provide a standard interface for exporting client functionality +type ModuleClients interface { + GetQueryCmd() *cobra.Command + GetTxCmd() *cobra.Command +} diff --git a/x/auth/client/cli/account.go b/x/auth/client/cli/account.go index f78b5252f..922b3e2db 100644 --- a/x/auth/client/cli/account.go +++ b/x/auth/client/cli/account.go @@ -5,34 +5,17 @@ import ( "github.com/spf13/cobra" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/context" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/auth" ) -// GetAccountCmdDefault invokes the GetAccountCmd for the auth.BaseAccount type. -func GetAccountCmdDefault(storeName string, cdc *codec.Codec) *cobra.Command { - return GetAccountCmd(storeName, cdc, GetAccountDecoder(cdc)) -} - -// GetAccountDecoder gets the account decoder for auth.DefaultAccount. -func GetAccountDecoder(cdc *codec.Codec) auth.AccountDecoder { - return func(accBytes []byte) (acct auth.Account, err error) { - err = cdc.UnmarshalBinaryBare(accBytes, &acct) - if err != nil { - panic(err) - } - - return acct, err - } -} - // GetAccountCmd returns a query account that will display the state of the // account at a given address. // nolint: unparam -func GetAccountCmd(storeName string, cdc *codec.Codec, decoder auth.AccountDecoder) *cobra.Command { - return &cobra.Command{ +func GetAccountCmd(storeName string, cdc *codec.Codec) *cobra.Command { + cmd := &cobra.Command{ Use: "account [address]", Short: "Query account balance", Args: cobra.ExactArgs(1), @@ -47,9 +30,9 @@ func GetAccountCmd(storeName string, cdc *codec.Codec, decoder auth.AccountDecod cliCtx := context.NewCLIContext(). WithCodec(cdc). - WithAccountDecoder(decoder) + WithAccountDecoder(cdc) - if err := cliCtx.EnsureAccountExistsFromAddr(key); err != nil { + if err = cliCtx.EnsureAccountExistsFromAddr(key); err != nil { return err } @@ -72,4 +55,7 @@ func GetAccountCmd(storeName string, cdc *codec.Codec, decoder auth.AccountDecod return nil }, } + + // Add the flags here and return the command + return client.GetCommands(cmd)[0] } diff --git a/x/auth/client/cli/sign.go b/x/auth/client/cli/sign.go index f4a4548d4..9075308e1 100644 --- a/x/auth/client/cli/sign.go +++ b/x/auth/client/cli/sign.go @@ -2,9 +2,10 @@ package cli import ( "fmt" + "io/ioutil" + "github.com/pkg/errors" "github.com/spf13/viper" - "io/ioutil" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/context" @@ -24,7 +25,7 @@ const ( ) // GetSignCommand returns the sign command -func GetSignCommand(codec *amino.Codec, decoder auth.AccountDecoder) *cobra.Command { +func GetSignCommand(codec *amino.Codec) *cobra.Command { cmd := &cobra.Command{ Use: "sign ", Short: "Sign transactions generated offline", @@ -41,7 +42,7 @@ order. The --offline flag makes sure that the client will not reach out to the local cache. Thus account number or sequence number lookups will not be performed and it is recommended to set such parameters manually.`, - RunE: makeSignCmd(codec, decoder), + RunE: makeSignCmd(codec), Args: cobra.ExactArgs(1), } cmd.Flags().String(client.FlagName, "", "Name of private key with which to sign") @@ -51,10 +52,12 @@ recommended to set such parameters manually.`, cmd.Flags().Bool(flagValidateSigs, false, "Print the addresses that must sign the transaction, "+ "those who have already signed it, and make sure that signatures are in the correct order.") cmd.Flags().Bool(flagOffline, false, "Offline mode. Do not query local cache.") - return cmd + + // Add the flags here and return the command + return client.PostCommands(cmd)[0] } -func makeSignCmd(cdc *amino.Codec, decoder auth.AccountDecoder) func(cmd *cobra.Command, args []string) error { +func makeSignCmd(cdc *amino.Codec) func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) (err error) { stdTx, err := readAndUnmarshalStdTx(cdc, args[0]) if err != nil { @@ -72,7 +75,7 @@ func makeSignCmd(cdc *amino.Codec, decoder auth.AccountDecoder) func(cmd *cobra. if name == "" { return errors.New("required flag \"name\" has not been set") } - cliCtx := context.NewCLIContext().WithCodec(cdc).WithAccountDecoder(decoder) + cliCtx := context.NewCLIContext().WithCodec(cdc).WithAccountDecoder(cdc) txBldr := authtxb.NewTxBuilderFromCLI() // if --signature-only is on, then override --append diff --git a/x/auth/client/rest/query.go b/x/auth/client/rest/query.go index 0629cc939..e60e0fdc1 100644 --- a/x/auth/client/rest/query.go +++ b/x/auth/client/rest/query.go @@ -8,7 +8,6 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth" - authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" "github.com/gorilla/mux" ) @@ -17,11 +16,11 @@ import ( func RegisterRoutes(cliCtx context.CLIContext, r *mux.Router, cdc *codec.Codec, storeName string) { r.HandleFunc( "/auth/accounts/{address}", - QueryAccountRequestHandlerFn(storeName, cdc, authcmd.GetAccountDecoder(cdc), cliCtx), + QueryAccountRequestHandlerFn(storeName, cdc, context.GetAccountDecoder(cdc), cliCtx), ).Methods("GET") r.HandleFunc( "/bank/balances/{address}", - QueryBalancesRequestHandlerFn(storeName, cdc, authcmd.GetAccountDecoder(cdc), cliCtx), + QueryBalancesRequestHandlerFn(storeName, cdc, context.GetAccountDecoder(cdc), cliCtx), ).Methods("GET") r.HandleFunc( "/tx/sign", diff --git a/x/bank/client/cli/broadcast.go b/x/bank/client/cli/broadcast.go index dd045439e..126668364 100644 --- a/x/bank/client/cli/broadcast.go +++ b/x/bank/client/cli/broadcast.go @@ -4,6 +4,7 @@ import ( "io/ioutil" "os" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/context" "github.com/cosmos/cosmos-sdk/x/auth" "github.com/spf13/cobra" @@ -36,7 +37,7 @@ in place of an input filename, the command reads from standard input.`, }, } - return cmd + return client.PostCommands(cmd)[0] } func readAndUnmarshalStdTx(cdc *amino.Codec, filename string) (stdTx auth.StdTx, err error) { diff --git a/x/bank/client/cli/sendtx.go b/x/bank/client/cli/sendtx.go index bae217976..29a101cf7 100644 --- a/x/bank/client/cli/sendtx.go +++ b/x/bank/client/cli/sendtx.go @@ -1,13 +1,13 @@ package cli import ( + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/context" "github.com/cosmos/cosmos-sdk/client/utils" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" authtxb "github.com/cosmos/cosmos-sdk/x/auth/client/txbuilder" - "github.com/cosmos/cosmos-sdk/x/bank/client" + bankClient "github.com/cosmos/cosmos-sdk/x/bank/client" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -28,7 +28,7 @@ func SendTxCmd(cdc *codec.Codec) *cobra.Command { txBldr := authtxb.NewTxBuilderFromCLI().WithCodec(cdc) cliCtx := context.NewCLIContext(). WithCodec(cdc). - WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) + WithAccountDecoder(cdc) if err := cliCtx.EnsureAccountExists(); err != nil { return err @@ -64,7 +64,7 @@ func SendTxCmd(cdc *codec.Codec) *cobra.Command { } // build and sign the transaction, then broadcast to Tendermint - msg := client.CreateMsg(from, to, coins) + msg := bankClient.CreateMsg(from, to, coins) if cliCtx.GenerateOnly { return utils.PrintUnsignedStdTx(txBldr, cliCtx, []sdk.Msg{msg}, false) } @@ -78,5 +78,5 @@ func SendTxCmd(cdc *codec.Codec) *cobra.Command { cmd.MarkFlagRequired(flagTo) cmd.MarkFlagRequired(flagAmount) - return cmd + return client.PostCommands(cmd)[0] } diff --git a/x/distribution/client/cli/tx.go b/x/distribution/client/cli/tx.go index b88968cda..ee82498e8 100644 --- a/x/distribution/client/cli/tx.go +++ b/x/distribution/client/cli/tx.go @@ -6,12 +6,13 @@ import ( "github.com/spf13/cobra" "github.com/spf13/viper" + amino "github.com/tendermint/go-amino" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/context" "github.com/cosmos/cosmos-sdk/client/utils" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" authtxb "github.com/cosmos/cosmos-sdk/x/auth/client/txbuilder" "github.com/cosmos/cosmos-sdk/x/distribution/types" @@ -22,6 +23,21 @@ var ( flagIsValidator = "is-validator" ) +// GetTxCmd returns the transaction commands for this module +func GetTxCmd(storeKey string, cdc *amino.Codec) *cobra.Command { + distTxCmd := &cobra.Command{ + Use: "dist", + Short: "Distribution transactions subcommands", + } + + distTxCmd.AddCommand(client.PostCommands( + GetCmdWithdrawRewards(cdc), + GetCmdSetWithdrawAddr(cdc), + )...) + + return distTxCmd +} + // command to withdraw rewards func GetCmdWithdrawRewards(cdc *codec.Codec) *cobra.Command { cmd := &cobra.Command{ @@ -41,7 +57,7 @@ func GetCmdWithdrawRewards(cdc *codec.Codec) *cobra.Command { txBldr := authtxb.NewTxBuilderFromCLI().WithCodec(cdc) cliCtx := context.NewCLIContext(). WithCodec(cdc). - WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) + WithAccountDecoder(cdc) var msg sdk.Msg switch { @@ -92,7 +108,7 @@ func GetCmdSetWithdrawAddr(cdc *codec.Codec) *cobra.Command { txBldr := authtxb.NewTxBuilderFromCLI().WithCodec(cdc) cliCtx := context.NewCLIContext(). WithCodec(cdc). - WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) + WithAccountDecoder(cdc) delAddr, err := cliCtx.GetFromAddress() if err != nil { diff --git a/x/distribution/client/module_client.go b/x/distribution/client/module_client.go new file mode 100644 index 000000000..ba725e1f8 --- /dev/null +++ b/x/distribution/client/module_client.go @@ -0,0 +1,38 @@ +package client + +import ( + "github.com/cosmos/cosmos-sdk/client" + distCmds "github.com/cosmos/cosmos-sdk/x/distribution/client/cli" + "github.com/spf13/cobra" + amino "github.com/tendermint/go-amino" +) + +// ModuleClient exports all client functionality from this module +type ModuleClient struct { + storeKey string + cdc *amino.Codec +} + +func NewModuleClient(storeKey string, cdc *amino.Codec) ModuleClient { + return ModuleClient{storeKey, cdc} +} + +// GetQueryCmd returns the cli query commands for this module +func (mc ModuleClient) GetQueryCmd() *cobra.Command { + return &cobra.Command{Hidden: true} +} + +// GetTxCmd returns the transaction commands for this module +func (mc ModuleClient) GetTxCmd() *cobra.Command { + distTxCmd := &cobra.Command{ + Use: "dist", + Short: "Distribution transactions subcommands", + } + + distTxCmd.AddCommand(client.PostCommands( + distCmds.GetCmdWithdrawRewards(mc.cdc), + distCmds.GetCmdSetWithdrawAddr(mc.cdc), + )...) + + return distTxCmd +} diff --git a/x/gov/client/cli/query.go b/x/gov/client/cli/query.go new file mode 100644 index 000000000..227f278a9 --- /dev/null +++ b/x/gov/client/cli/query.go @@ -0,0 +1,324 @@ +package cli + +import ( + "fmt" + + "github.com/cosmos/cosmos-sdk/client/context" + "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/gov" + govClientUtils "github.com/cosmos/cosmos-sdk/x/gov/client/utils" + "github.com/spf13/cobra" + "github.com/spf13/viper" +) + +// GetCmdQueryProposal implements the query proposal command. +func GetCmdQueryProposal(queryRoute string, cdc *codec.Codec) *cobra.Command { + cmd := &cobra.Command{ + Use: "proposal", + Short: "Query details of a single proposal", + RunE: func(cmd *cobra.Command, args []string) error { + cliCtx := context.NewCLIContext().WithCodec(cdc) + proposalID := uint64(viper.GetInt64(flagProposalID)) + + params := gov.QueryProposalParams{ + ProposalID: proposalID, + } + + bz, err := cdc.MarshalJSON(params) + if err != nil { + return err + } + + res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/proposal", queryRoute), bz) + if err != nil { + return err + } + + fmt.Println(string(res)) + return nil + }, + } + + cmd.Flags().String(flagProposalID, "", "proposalID of proposal being queried") + + return cmd +} + +// GetCmdQueryProposals implements a query proposals command. +func GetCmdQueryProposals(queryRoute string, cdc *codec.Codec) *cobra.Command { + cmd := &cobra.Command{ + Use: "proposals", + Short: "Query proposals with optional filters", + RunE: func(cmd *cobra.Command, args []string) error { + bechDepositerAddr := viper.GetString(flagDepositer) + bechVoterAddr := viper.GetString(flagVoter) + strProposalStatus := viper.GetString(flagStatus) + numLimit := uint64(viper.GetInt64(flagNumLimit)) + + params := gov.QueryProposalsParams{ + Limit: numLimit, + } + + if len(bechDepositerAddr) != 0 { + depositerAddr, err := sdk.AccAddressFromBech32(bechDepositerAddr) + if err != nil { + return err + } + params.Depositer = depositerAddr + } + + if len(bechVoterAddr) != 0 { + voterAddr, err := sdk.AccAddressFromBech32(bechVoterAddr) + if err != nil { + return err + } + params.Voter = voterAddr + } + + if len(strProposalStatus) != 0 { + proposalStatus, err := gov.ProposalStatusFromString(govClientUtils.NormalizeProposalStatus(strProposalStatus)) + if err != nil { + return err + } + params.ProposalStatus = proposalStatus + } + + bz, err := cdc.MarshalJSON(params) + if err != nil { + return err + } + + cliCtx := context.NewCLIContext().WithCodec(cdc) + + res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/proposals", queryRoute), bz) + if err != nil { + return err + } + + var matchingProposals []gov.Proposal + err = cdc.UnmarshalJSON(res, &matchingProposals) + if err != nil { + return err + } + + if len(matchingProposals) == 0 { + fmt.Println("No matching proposals found") + return nil + } + + for _, proposal := range matchingProposals { + fmt.Printf(" %d - %s\n", proposal.GetProposalID(), proposal.GetTitle()) + } + + return nil + }, + } + + cmd.Flags().String(flagNumLimit, "", "(optional) limit to latest [number] proposals. Defaults to all proposals") + cmd.Flags().String(flagDepositer, "", "(optional) filter by proposals deposited on by depositer") + cmd.Flags().String(flagVoter, "", "(optional) filter by proposals voted on by voted") + cmd.Flags().String(flagStatus, "", "(optional) filter proposals by proposal status, status: deposit_period/voting_period/passed/rejected") + + return cmd +} + +// Command to Get a Proposal Information +// GetCmdQueryVote implements the query proposal vote command. +func GetCmdQueryVote(queryRoute string, cdc *codec.Codec) *cobra.Command { + cmd := &cobra.Command{ + Use: "vote", + Short: "Query details of a single vote", + RunE: func(cmd *cobra.Command, args []string) error { + cliCtx := context.NewCLIContext().WithCodec(cdc) + proposalID := uint64(viper.GetInt64(flagProposalID)) + + voterAddr, err := sdk.AccAddressFromBech32(viper.GetString(flagVoter)) + if err != nil { + return err + } + + params := gov.QueryVoteParams{ + Voter: voterAddr, + ProposalID: proposalID, + } + bz, err := cdc.MarshalJSON(params) + if err != nil { + return err + } + + res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/vote", queryRoute), bz) + if err != nil { + return err + } + + fmt.Println(string(res)) + return nil + }, + } + + cmd.Flags().String(flagProposalID, "", "proposalID of proposal voting on") + cmd.Flags().String(flagVoter, "", "bech32 voter address") + + return cmd +} + +// GetCmdQueryVotes implements the command to query for proposal votes. +func GetCmdQueryVotes(queryRoute string, cdc *codec.Codec) *cobra.Command { + cmd := &cobra.Command{ + Use: "votes", + Short: "Query votes on a proposal", + RunE: func(cmd *cobra.Command, args []string) error { + cliCtx := context.NewCLIContext().WithCodec(cdc) + proposalID := uint64(viper.GetInt64(flagProposalID)) + + params := gov.QueryVotesParams{ + ProposalID: proposalID, + } + bz, err := cdc.MarshalJSON(params) + if err != nil { + return err + } + + res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/votes", queryRoute), bz) + if err != nil { + return err + } + + fmt.Println(string(res)) + return nil + }, + } + + cmd.Flags().String(flagProposalID, "", "proposalID of which proposal's votes are being queried") + + return cmd +} + +// Command to Get a specific Deposit Information +// GetCmdQueryDeposit implements the query proposal deposit command. +func GetCmdQueryDeposit(queryRoute string, cdc *codec.Codec) *cobra.Command { + cmd := &cobra.Command{ + Use: "deposit", + Short: "Query details of a deposit", + RunE: func(cmd *cobra.Command, args []string) error { + cliCtx := context.NewCLIContext().WithCodec(cdc) + proposalID := uint64(viper.GetInt64(flagProposalID)) + + depositerAddr, err := sdk.AccAddressFromBech32(viper.GetString(flagDepositer)) + if err != nil { + return err + } + + params := gov.QueryDepositParams{ + Depositer: depositerAddr, + ProposalID: proposalID, + } + bz, err := cdc.MarshalJSON(params) + if err != nil { + return err + } + + res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/deposit", queryRoute), bz) + if err != nil { + return err + } + + fmt.Println(string(res)) + return nil + }, + } + + cmd.Flags().String(flagProposalID, "", "proposalID of proposal deposited on") + cmd.Flags().String(flagDepositer, "", "bech32 depositer address") + + return cmd +} + +// GetCmdQueryDeposits implements the command to query for proposal deposits. +func GetCmdQueryDeposits(queryRoute string, cdc *codec.Codec) *cobra.Command { + cmd := &cobra.Command{ + Use: "deposits", + Short: "Query deposits on a proposal", + RunE: func(cmd *cobra.Command, args []string) error { + cliCtx := context.NewCLIContext().WithCodec(cdc) + proposalID := uint64(viper.GetInt64(flagProposalID)) + + params := gov.QueryDepositsParams{ + ProposalID: proposalID, + } + bz, err := cdc.MarshalJSON(params) + if err != nil { + return err + } + + res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/deposits", queryRoute), bz) + if err != nil { + return err + } + + fmt.Println(string(res)) + return nil + }, + } + + cmd.Flags().String(flagProposalID, "", "proposalID of which proposal's deposits are being queried") + + return cmd +} + +// GetCmdQueryTally implements the command to query for proposal tally result. +func GetCmdQueryTally(queryRoute string, cdc *codec.Codec) *cobra.Command { + cmd := &cobra.Command{ + Use: "tally", + Short: "Get the tally of a proposal vote", + RunE: func(cmd *cobra.Command, args []string) error { + cliCtx := context.NewCLIContext().WithCodec(cdc) + proposalID := uint64(viper.GetInt64(flagProposalID)) + + params := gov.QueryTallyParams{ + ProposalID: proposalID, + } + bz, err := cdc.MarshalJSON(params) + if err != nil { + return err + } + + res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/tally", queryRoute), bz) + if err != nil { + return err + } + + fmt.Println(string(res)) + return nil + }, + } + + cmd.Flags().String(flagProposalID, "", "proposalID of which proposal is being tallied") + + return cmd +} + +// GetCmdQueryProposal implements the query proposal command. +func GetCmdQueryParams(queryRoute string, cdc *codec.Codec) *cobra.Command { + cmd := &cobra.Command{ + Use: "param [param-type]", + Short: "Query the parameters (voting|tallying|deposit) of the governance process", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + paramType := args[0] + + cliCtx := context.NewCLIContext().WithCodec(cdc) + + res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/params/%s", queryRoute, paramType), nil) + if err != nil { + return err + } + + fmt.Println(string(res)) + return nil + }, + } + + return cmd +} diff --git a/x/gov/client/cli/tx.go b/x/gov/client/cli/tx.go index 31780f68e..e804863d1 100644 --- a/x/gov/client/cli/tx.go +++ b/x/gov/client/cli/tx.go @@ -7,7 +7,6 @@ import ( "github.com/cosmos/cosmos-sdk/client/utils" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" authtxb "github.com/cosmos/cosmos-sdk/x/auth/client/txbuilder" "github.com/cosmos/cosmos-sdk/x/gov" @@ -15,7 +14,7 @@ import ( "io/ioutil" "strings" - "github.com/cosmos/cosmos-sdk/x/gov/client" + govClientUtils "github.com/cosmos/cosmos-sdk/x/gov/client/utils" "github.com/spf13/cobra" "github.com/spf13/viper" ) @@ -80,7 +79,7 @@ $ gaiacli gov submit-proposal --title="Test Proposal" --description="My awesome txBldr := authtxb.NewTxBuilderFromCLI().WithCodec(cdc) cliCtx := context.NewCLIContext(). WithCodec(cdc). - WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) + WithAccountDecoder(cdc) fromAddr, err := cliCtx.GetFromAddress() if err != nil { @@ -130,7 +129,7 @@ func parseSubmitProposalFlags() (*proposal, error) { if proposalFile == "" { proposal.Title = viper.GetString(flagTitle) proposal.Description = viper.GetString(flagDescription) - proposal.Type = client.NormalizeProposalType(viper.GetString(flagProposalType)) + proposal.Type = govClientUtils.NormalizeProposalType(viper.GetString(flagProposalType)) proposal.Deposit = viper.GetString(flagDeposit) return proposal, nil } @@ -163,7 +162,7 @@ func GetCmdDeposit(cdc *codec.Codec) *cobra.Command { txBldr := authtxb.NewTxBuilderFromCLI().WithCodec(cdc) cliCtx := context.NewCLIContext(). WithCodec(cdc). - WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) + WithAccountDecoder(cdc) depositerAddr, err := cliCtx.GetFromAddress() if err != nil { @@ -208,7 +207,7 @@ func GetCmdVote(cdc *codec.Codec) *cobra.Command { txBldr := authtxb.NewTxBuilderFromCLI().WithCodec(cdc) cliCtx := context.NewCLIContext(). WithCodec(cdc). - WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) + WithAccountDecoder(cdc) voterAddr, err := cliCtx.GetFromAddress() if err != nil { @@ -218,7 +217,7 @@ func GetCmdVote(cdc *codec.Codec) *cobra.Command { proposalID := uint64(viper.GetInt64(flagProposalID)) option := viper.GetString(flagOption) - byteVoteOption, err := gov.VoteOptionFromString(client.NormalizeVoteOption(option)) + byteVoteOption, err := gov.VoteOptionFromString(govClientUtils.NormalizeVoteOption(option)) if err != nil { return err } @@ -248,314 +247,3 @@ func GetCmdVote(cdc *codec.Codec) *cobra.Command { return cmd } - -// GetCmdQueryProposal implements the query proposal command. -func GetCmdQueryParams(queryRoute string, cdc *codec.Codec) *cobra.Command { - cmd := &cobra.Command{ - Use: "param [param-type]", - Short: "Query the parameters (voting|tallying|deposit) of the governance process", - Args: cobra.ExactArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - paramType := args[0] - - cliCtx := context.NewCLIContext().WithCodec(cdc) - - res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/params/%s", queryRoute, paramType), nil) - if err != nil { - return err - } - - fmt.Println(string(res)) - return nil - }, - } - - return cmd -} - -// GetCmdQueryProposal implements the query proposal command. -func GetCmdQueryProposal(queryRoute string, cdc *codec.Codec) *cobra.Command { - cmd := &cobra.Command{ - Use: "proposal", - Short: "Query details of a single proposal", - RunE: func(cmd *cobra.Command, args []string) error { - cliCtx := context.NewCLIContext().WithCodec(cdc) - proposalID := uint64(viper.GetInt64(flagProposalID)) - - params := gov.QueryProposalParams{ - ProposalID: proposalID, - } - - bz, err := cdc.MarshalJSON(params) - if err != nil { - return err - } - - res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/proposal", queryRoute), bz) - if err != nil { - return err - } - - fmt.Println(string(res)) - return nil - }, - } - - cmd.Flags().String(flagProposalID, "", "proposalID of proposal being queried") - - return cmd -} - -// GetCmdQueryProposals implements a query proposals command. -func GetCmdQueryProposals(queryRoute string, cdc *codec.Codec) *cobra.Command { - cmd := &cobra.Command{ - Use: "proposals", - Short: "Query proposals with optional filters", - RunE: func(cmd *cobra.Command, args []string) error { - bechDepositerAddr := viper.GetString(flagDepositer) - bechVoterAddr := viper.GetString(flagVoter) - strProposalStatus := viper.GetString(flagStatus) - numLimit := uint64(viper.GetInt64(flagNumLimit)) - - params := gov.QueryProposalsParams{ - Limit: numLimit, - } - - if len(bechDepositerAddr) != 0 { - depositerAddr, err := sdk.AccAddressFromBech32(bechDepositerAddr) - if err != nil { - return err - } - params.Depositer = depositerAddr - } - - if len(bechVoterAddr) != 0 { - voterAddr, err := sdk.AccAddressFromBech32(bechVoterAddr) - if err != nil { - return err - } - params.Voter = voterAddr - } - - if len(strProposalStatus) != 0 { - proposalStatus, err := gov.ProposalStatusFromString(client.NormalizeProposalStatus(strProposalStatus)) - if err != nil { - return err - } - params.ProposalStatus = proposalStatus - } - - bz, err := cdc.MarshalJSON(params) - if err != nil { - return err - } - - cliCtx := context.NewCLIContext().WithCodec(cdc) - - res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/proposals", queryRoute), bz) - if err != nil { - return err - } - - var matchingProposals []gov.Proposal - err = cdc.UnmarshalJSON(res, &matchingProposals) - if err != nil { - return err - } - - if len(matchingProposals) == 0 { - fmt.Println("No matching proposals found") - return nil - } - - for _, proposal := range matchingProposals { - fmt.Printf(" %d - %s\n", proposal.GetProposalID(), proposal.GetTitle()) - } - - return nil - }, - } - - cmd.Flags().String(flagNumLimit, "", "(optional) limit to latest [number] proposals. Defaults to all proposals") - cmd.Flags().String(flagDepositer, "", "(optional) filter by proposals deposited on by depositer") - cmd.Flags().String(flagVoter, "", "(optional) filter by proposals voted on by voted") - cmd.Flags().String(flagStatus, "", "(optional) filter proposals by proposal status, status: deposit_period/voting_period/passed/rejected") - - return cmd -} - -// Command to Get a Proposal Information -// GetCmdQueryVote implements the query proposal vote command. -func GetCmdQueryVote(queryRoute string, cdc *codec.Codec) *cobra.Command { - cmd := &cobra.Command{ - Use: "vote", - Short: "Query details of a single vote", - RunE: func(cmd *cobra.Command, args []string) error { - cliCtx := context.NewCLIContext().WithCodec(cdc) - proposalID := uint64(viper.GetInt64(flagProposalID)) - - voterAddr, err := sdk.AccAddressFromBech32(viper.GetString(flagVoter)) - if err != nil { - return err - } - - params := gov.QueryVoteParams{ - Voter: voterAddr, - ProposalID: proposalID, - } - bz, err := cdc.MarshalJSON(params) - if err != nil { - return err - } - - res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/vote", queryRoute), bz) - if err != nil { - return err - } - - fmt.Println(string(res)) - return nil - }, - } - - cmd.Flags().String(flagProposalID, "", "proposalID of proposal voting on") - cmd.Flags().String(flagVoter, "", "bech32 voter address") - - return cmd -} - -// GetCmdQueryVotes implements the command to query for proposal votes. -func GetCmdQueryVotes(queryRoute string, cdc *codec.Codec) *cobra.Command { - cmd := &cobra.Command{ - Use: "votes", - Short: "Query votes on a proposal", - RunE: func(cmd *cobra.Command, args []string) error { - cliCtx := context.NewCLIContext().WithCodec(cdc) - proposalID := uint64(viper.GetInt64(flagProposalID)) - - params := gov.QueryVotesParams{ - ProposalID: proposalID, - } - bz, err := cdc.MarshalJSON(params) - if err != nil { - return err - } - - res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/votes", queryRoute), bz) - if err != nil { - return err - } - - fmt.Println(string(res)) - return nil - }, - } - - cmd.Flags().String(flagProposalID, "", "proposalID of which proposal's votes are being queried") - - return cmd -} - -// Command to Get a specific Deposit Information -// GetCmdQueryDeposit implements the query proposal deposit command. -func GetCmdQueryDeposit(queryRoute string, cdc *codec.Codec) *cobra.Command { - cmd := &cobra.Command{ - Use: "deposit", - Short: "Query details of a deposit", - RunE: func(cmd *cobra.Command, args []string) error { - cliCtx := context.NewCLIContext().WithCodec(cdc) - proposalID := uint64(viper.GetInt64(flagProposalID)) - - depositerAddr, err := sdk.AccAddressFromBech32(viper.GetString(flagDepositer)) - if err != nil { - return err - } - - params := gov.QueryDepositParams{ - Depositer: depositerAddr, - ProposalID: proposalID, - } - bz, err := cdc.MarshalJSON(params) - if err != nil { - return err - } - - res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/deposit", queryRoute), bz) - if err != nil { - return err - } - - fmt.Println(string(res)) - return nil - }, - } - - cmd.Flags().String(flagProposalID, "", "proposalID of proposal deposited on") - cmd.Flags().String(flagDepositer, "", "bech32 depositer address") - - return cmd -} - -// GetCmdQueryDeposits implements the command to query for proposal deposits. -func GetCmdQueryDeposits(queryRoute string, cdc *codec.Codec) *cobra.Command { - cmd := &cobra.Command{ - Use: "deposits", - Short: "Query deposits on a proposal", - RunE: func(cmd *cobra.Command, args []string) error { - cliCtx := context.NewCLIContext().WithCodec(cdc) - proposalID := uint64(viper.GetInt64(flagProposalID)) - - params := gov.QueryDepositsParams{ - ProposalID: proposalID, - } - bz, err := cdc.MarshalJSON(params) - if err != nil { - return err - } - - res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/deposits", queryRoute), bz) - if err != nil { - return err - } - - fmt.Println(string(res)) - return nil - }, - } - - cmd.Flags().String(flagProposalID, "", "proposalID of which proposal's deposits are being queried") - - return cmd -} - -// GetCmdQueryTally implements the command to query for proposal tally result. -func GetCmdQueryTally(queryRoute string, cdc *codec.Codec) *cobra.Command { - cmd := &cobra.Command{ - Use: "tally", - Short: "Get the tally of a proposal vote", - RunE: func(cmd *cobra.Command, args []string) error { - cliCtx := context.NewCLIContext().WithCodec(cdc) - proposalID := uint64(viper.GetInt64(flagProposalID)) - - params := gov.QueryTallyParams{ - ProposalID: proposalID, - } - bz, err := cdc.MarshalJSON(params) - if err != nil { - return err - } - - res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/tally", queryRoute), bz) - if err != nil { - return err - } - - fmt.Println(string(res)) - return nil - }, - } - - cmd.Flags().String(flagProposalID, "", "proposalID of which proposal is being tallied") - - return cmd -} diff --git a/x/gov/client/module_client.go b/x/gov/client/module_client.go new file mode 100644 index 000000000..7cabc6884 --- /dev/null +++ b/x/gov/client/module_client.go @@ -0,0 +1,55 @@ +package client + +import ( + "github.com/cosmos/cosmos-sdk/client" + govCli "github.com/cosmos/cosmos-sdk/x/gov/client/cli" + "github.com/spf13/cobra" + amino "github.com/tendermint/go-amino" +) + +// ModuleClient exports all client functionality from this module +type ModuleClient struct { + storeKey string + cdc *amino.Codec +} + +func NewModuleClient(storeKey string, cdc *amino.Codec) ModuleClient { + return ModuleClient{storeKey, cdc} +} + +// GetQueryCmd returns the cli query commands for this module +func (mc ModuleClient) GetQueryCmd() *cobra.Command { + // Group gov queries under a subcommand + govQueryCmd := &cobra.Command{ + Use: "gov", + Short: "Querying commands for the governance module", + } + + govQueryCmd.AddCommand(client.GetCommands( + govCli.GetCmdQueryProposal(mc.storeKey, mc.cdc), + govCli.GetCmdQueryProposals(mc.storeKey, mc.cdc), + govCli.GetCmdQueryVote(mc.storeKey, mc.cdc), + govCli.GetCmdQueryVotes(mc.storeKey, mc.cdc), + govCli.GetCmdQueryParams(mc.storeKey, mc.cdc), + govCli.GetCmdQueryDeposit(mc.storeKey, mc.cdc), + govCli.GetCmdQueryDeposits(mc.storeKey, mc.cdc), + govCli.GetCmdQueryTally(mc.storeKey, mc.cdc))...) + + return govQueryCmd +} + +// GetTxCmd returns the transaction commands for this module +func (mc ModuleClient) GetTxCmd() *cobra.Command { + govTxCmd := &cobra.Command{ + Use: "gov", + Short: "Governance transactions subcommands", + } + + govTxCmd.AddCommand(client.PostCommands( + govCli.GetCmdDeposit(mc.cdc), + govCli.GetCmdVote(mc.cdc), + govCli.GetCmdSubmitProposal(mc.cdc), + )...) + + return govTxCmd +} diff --git a/x/gov/client/rest/rest.go b/x/gov/client/rest/rest.go index 930a39b02..c000a34d8 100644 --- a/x/gov/client/rest/rest.go +++ b/x/gov/client/rest/rest.go @@ -10,7 +10,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/gov" - "github.com/cosmos/cosmos-sdk/x/gov/client" + govClientUtils "github.com/cosmos/cosmos-sdk/x/gov/client/utils" "github.com/gorilla/mux" "github.com/pkg/errors" ) @@ -81,7 +81,7 @@ func postProposalHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.Han return } - proposalType, err := gov.ProposalTypeFromString(client.NormalizeProposalType(req.ProposalType)) + proposalType, err := gov.ProposalTypeFromString(govClientUtils.NormalizeProposalType(req.ProposalType)) if err != nil { utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return @@ -165,7 +165,7 @@ func voteHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.HandlerFunc return } - voteOption, err := gov.VoteOptionFromString(client.NormalizeVoteOption(req.Option)) + voteOption, err := gov.VoteOptionFromString(govClientUtils.NormalizeVoteOption(req.Option)) if err != nil { utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return @@ -460,7 +460,7 @@ func queryProposalsWithParameterFn(cdc *codec.Codec, cliCtx context.CLIContext) } if len(strProposalStatus) != 0 { - proposalStatus, err := gov.ProposalStatusFromString(client.NormalizeProposalStatus(strProposalStatus)) + proposalStatus, err := gov.ProposalStatusFromString(govClientUtils.NormalizeProposalStatus(strProposalStatus)) if err != nil { utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return diff --git a/x/gov/client/utils.go b/x/gov/client/utils/utils.go similarity index 98% rename from x/gov/client/utils.go rename to x/gov/client/utils/utils.go index 013f3944a..e91d7005c 100644 --- a/x/gov/client/utils.go +++ b/x/gov/client/utils/utils.go @@ -1,4 +1,4 @@ -package client +package utils // NormalizeVoteOption - normalize user specified vote option func NormalizeVoteOption(option string) string { diff --git a/x/ibc/client/cli/ibctx.go b/x/ibc/client/cli/ibctx.go index be6dfc940..e8b107d9a 100644 --- a/x/ibc/client/cli/ibctx.go +++ b/x/ibc/client/cli/ibctx.go @@ -8,7 +8,6 @@ import ( "github.com/cosmos/cosmos-sdk/client/utils" codec "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" authtxb "github.com/cosmos/cosmos-sdk/x/auth/client/txbuilder" "github.com/cosmos/cosmos-sdk/x/ibc" @@ -30,7 +29,7 @@ func IBCTransferCmd(cdc *codec.Codec) *cobra.Command { txBldr := authtxb.NewTxBuilderFromCLI().WithCodec(cdc) cliCtx := context.NewCLIContext(). WithCodec(cdc). - WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) + WithAccountDecoder(cdc) from, err := cliCtx.GetFromAddress() if err != nil { diff --git a/x/ibc/client/cli/relay.go b/x/ibc/client/cli/relay.go index 43ad783f4..217d7cdee 100644 --- a/x/ibc/client/cli/relay.go +++ b/x/ibc/client/cli/relay.go @@ -9,7 +9,6 @@ import ( codec "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth" - authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" authtxb "github.com/cosmos/cosmos-sdk/x/auth/client/txbuilder" "github.com/cosmos/cosmos-sdk/x/ibc" @@ -42,7 +41,7 @@ type relayCommander struct { func IBCRelayCmd(cdc *codec.Codec) *cobra.Command { cmdr := relayCommander{ cdc: cdc, - decoder: authcmd.GetAccountDecoder(cdc), + decoder: context.GetAccountDecoder(cdc), ibcStore: "ibc", mainStore: "main", accStore: "acc", diff --git a/x/slashing/client/cli/tx.go b/x/slashing/client/cli/tx.go index f70be1871..7124b3544 100644 --- a/x/slashing/client/cli/tx.go +++ b/x/slashing/client/cli/tx.go @@ -5,7 +5,6 @@ import ( "github.com/cosmos/cosmos-sdk/client/utils" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" authtxb "github.com/cosmos/cosmos-sdk/x/auth/client/txbuilder" "github.com/cosmos/cosmos-sdk/x/slashing" @@ -22,7 +21,7 @@ func GetCmdUnjail(cdc *codec.Codec) *cobra.Command { txBldr := authtxb.NewTxBuilderFromCLI().WithCodec(cdc) cliCtx := context.NewCLIContext(). WithCodec(cdc). - WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) + WithAccountDecoder(cdc) valAddr, err := cliCtx.GetFromAddress() if err != nil { diff --git a/x/slashing/client/module_client.go b/x/slashing/client/module_client.go new file mode 100644 index 000000000..82efb5afe --- /dev/null +++ b/x/slashing/client/module_client.go @@ -0,0 +1,47 @@ +package client + +import ( + "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/x/slashing/client/cli" + "github.com/spf13/cobra" + amino "github.com/tendermint/go-amino" +) + +// ModuleClient exports all client functionality from this module +type ModuleClient struct { + storeKey string + cdc *amino.Codec +} + +func NewModuleClient(storeKey string, cdc *amino.Codec) ModuleClient { + return ModuleClient{storeKey, cdc} +} + +// GetQueryCmd returns the cli query commands for this module +func (mc ModuleClient) GetQueryCmd() *cobra.Command { + // Group slashing queries under a subcommand + slashingQueryCmd := &cobra.Command{ + Use: "slashing", + Short: "Querying commands for the slashing module", + } + + slashingQueryCmd.AddCommand(client.GetCommands( + cli.GetCmdQuerySigningInfo(mc.storeKey, mc.cdc))...) + + return slashingQueryCmd + +} + +// GetTxCmd returns the transaction commands for this module +func (mc ModuleClient) GetTxCmd() *cobra.Command { + slashingTxCmd := &cobra.Command{ + Use: "slashing", + Short: "Slashing transactions subcommands", + } + + slashingTxCmd.AddCommand(client.PostCommands( + cli.GetCmdUnjail(mc.cdc), + )...) + + return slashingTxCmd +} diff --git a/x/stake/client/cli/query.go b/x/stake/client/cli/query.go index 24e449996..40598fcf8 100644 --- a/x/stake/client/cli/query.go +++ b/x/stake/client/cli/query.go @@ -115,7 +115,7 @@ func GetCmdQueryValidators(storeName string, cdc *codec.Codec) *cobra.Command { } // GetCmdQueryValidatorUnbondingDelegations implements the query all unbonding delegatations from a validator command. -func GetCmdQueryValidatorUnbondingDelegations(queryRoute string, cdc *codec.Codec) *cobra.Command { +func GetCmdQueryValidatorUnbondingDelegations(storeKey string, cdc *codec.Codec) *cobra.Command { cmd := &cobra.Command{ Use: "unbonding-delegations-from [operator-addr]", Short: "Query all unbonding delegatations from a validator", @@ -135,7 +135,7 @@ func GetCmdQueryValidatorUnbondingDelegations(queryRoute string, cdc *codec.Code } res, err := cliCtx.QueryWithData( - fmt.Sprintf("custom/%s/validatorUnbondingDelegations", queryRoute), + fmt.Sprintf("custom/%s/validatorUnbondingDelegations", storeKey), bz) if err != nil { return err @@ -150,7 +150,7 @@ func GetCmdQueryValidatorUnbondingDelegations(queryRoute string, cdc *codec.Code } // GetCmdQueryValidatorRedelegations implements the query all redelegatations from a validator command. -func GetCmdQueryValidatorRedelegations(queryRoute string, cdc *codec.Codec) *cobra.Command { +func GetCmdQueryValidatorRedelegations(storeKey string, cdc *codec.Codec) *cobra.Command { cmd := &cobra.Command{ Use: "redelegations-from [operator-addr]", Short: "Query all outgoing redelegatations from a validator", @@ -170,7 +170,7 @@ func GetCmdQueryValidatorRedelegations(queryRoute string, cdc *codec.Codec) *cob } res, err := cliCtx.QueryWithData( - fmt.Sprintf("custom/%s/validatorRedelegations", queryRoute), + fmt.Sprintf("custom/%s/validatorRedelegations", storeKey), bz) if err != nil { return err @@ -288,7 +288,7 @@ func GetCmdQueryDelegations(storeName string, cdc *codec.Codec) *cobra.Command { // GetCmdQueryValidatorDelegations implements the command to query all the // delegations to a specific validator. -func GetCmdQueryValidatorDelegations(queryRoute string, cdc *codec.Codec) *cobra.Command { +func GetCmdQueryValidatorDelegations(storeKey string, cdc *codec.Codec) *cobra.Command { cmd := &cobra.Command{ Use: "delegations-to [validator-addr]", Short: "Query all delegations made to one validator", @@ -308,7 +308,7 @@ func GetCmdQueryValidatorDelegations(queryRoute string, cdc *codec.Codec) *cobra cliCtx := context.NewCLIContext().WithCodec(cdc) - res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/validatorDelegations", queryRoute), bz) + res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/validatorDelegations", storeKey), bz) if err != nil { return err } diff --git a/x/stake/client/cli/tx.go b/x/stake/client/cli/tx.go index a42fcf6d0..09b235abb 100644 --- a/x/stake/client/cli/tx.go +++ b/x/stake/client/cli/tx.go @@ -8,7 +8,6 @@ import ( "github.com/cosmos/cosmos-sdk/client/utils" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" authtxb "github.com/cosmos/cosmos-sdk/x/auth/client/txbuilder" "github.com/cosmos/cosmos-sdk/x/stake" @@ -25,7 +24,7 @@ func GetCmdCreateValidator(cdc *codec.Codec) *cobra.Command { txBldr := authtxb.NewTxBuilderFromCLI().WithCodec(cdc) cliCtx := context.NewCLIContext(). WithCodec(cdc). - WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) + WithAccountDecoder(cdc) amounstStr := viper.GetString(FlagAmount) if amounstStr == "" { @@ -126,7 +125,7 @@ func GetCmdEditValidator(cdc *codec.Codec) *cobra.Command { txBldr := authtxb.NewTxBuilderFromCLI().WithCodec(cdc) cliCtx := context.NewCLIContext(). WithCodec(cdc). - WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) + WithAccountDecoder(cdc) valAddr, err := cliCtx.GetFromAddress() if err != nil { @@ -178,7 +177,7 @@ func GetCmdDelegate(cdc *codec.Codec) *cobra.Command { txBldr := authtxb.NewTxBuilderFromCLI().WithCodec(cdc) cliCtx := context.NewCLIContext(). WithCodec(cdc). - WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) + WithAccountDecoder(cdc) amount, err := sdk.ParseCoin(viper.GetString(FlagAmount)) if err != nil { @@ -220,7 +219,7 @@ func GetCmdRedelegate(storeName string, cdc *codec.Codec) *cobra.Command { txBldr := authtxb.NewTxBuilderFromCLI().WithCodec(cdc) cliCtx := context.NewCLIContext(). WithCodec(cdc). - WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) + WithAccountDecoder(cdc) var err error @@ -275,7 +274,7 @@ func GetCmdUnbond(storeName string, cdc *codec.Codec) *cobra.Command { txBldr := authtxb.NewTxBuilderFromCLI().WithCodec(cdc) cliCtx := context.NewCLIContext(). WithCodec(cdc). - WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) + WithAccountDecoder(cdc) delAddr, err := cliCtx.GetFromAddress() if err != nil { diff --git a/x/stake/client/cli/utils.go b/x/stake/client/cli/utils.go index 9aca2d899..502cb11ec 100644 --- a/x/stake/client/cli/utils.go +++ b/x/stake/client/cli/utils.go @@ -4,7 +4,6 @@ import ( "github.com/cosmos/cosmos-sdk/client/context" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" "github.com/cosmos/cosmos-sdk/x/stake" "github.com/cosmos/cosmos-sdk/x/stake/types" "github.com/pkg/errors" @@ -45,7 +44,7 @@ func getShares( key := stake.GetDelegationKey(delAddr, valAddr) cliCtx := context.NewCLIContext(). WithCodec(cdc). - WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) + WithAccountDecoder(cdc) resQuery, err := cliCtx.QueryStore(key, storeName) if err != nil { diff --git a/x/stake/client/module_client.go b/x/stake/client/module_client.go new file mode 100644 index 000000000..5a08668dc --- /dev/null +++ b/x/stake/client/module_client.go @@ -0,0 +1,61 @@ +package client + +import ( + "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/x/stake/client/cli" + "github.com/spf13/cobra" + amino "github.com/tendermint/go-amino" +) + +// ModuleClient exports all client functionality from this module +type ModuleClient struct { + storeKey string + cdc *amino.Codec +} + +func NewModuleClient(storeKey string, cdc *amino.Codec) ModuleClient { + return ModuleClient{storeKey, cdc} +} + +// GetQueryCmd returns the cli query commands for this module +func (mc ModuleClient) GetQueryCmd() *cobra.Command { + stakeQueryCmd := &cobra.Command{ + Use: "stake", + Short: "Querying commands for the staking module", + } + stakeQueryCmd.AddCommand(client.GetCommands( + cli.GetCmdQueryDelegation(mc.storeKey, mc.cdc), + cli.GetCmdQueryDelegations(mc.storeKey, mc.cdc), + cli.GetCmdQueryUnbondingDelegation(mc.storeKey, mc.cdc), + cli.GetCmdQueryUnbondingDelegations(mc.storeKey, mc.cdc), + cli.GetCmdQueryRedelegation(mc.storeKey, mc.cdc), + cli.GetCmdQueryRedelegations(mc.storeKey, mc.cdc), + cli.GetCmdQueryValidator(mc.storeKey, mc.cdc), + cli.GetCmdQueryValidators(mc.storeKey, mc.cdc), + cli.GetCmdQueryValidatorDelegations(mc.storeKey, mc.cdc), + cli.GetCmdQueryValidatorUnbondingDelegations(mc.storeKey, mc.cdc), + cli.GetCmdQueryValidatorRedelegations(mc.storeKey, mc.cdc), + cli.GetCmdQueryParams(mc.storeKey, mc.cdc), + cli.GetCmdQueryPool(mc.storeKey, mc.cdc))...) + + return stakeQueryCmd + +} + +// GetTxCmd returns the transaction commands for this module +func (mc ModuleClient) GetTxCmd() *cobra.Command { + stakeTxCmd := &cobra.Command{ + Use: "stake", + Short: "Staking transaction subcommands", + } + + stakeTxCmd.AddCommand(client.PostCommands( + cli.GetCmdCreateValidator(mc.cdc), + cli.GetCmdEditValidator(mc.cdc), + cli.GetCmdDelegate(mc.cdc), + cli.GetCmdRedelegate(mc.storeKey, mc.cdc), + cli.GetCmdUnbond(mc.storeKey, mc.cdc), + )...) + + return stakeTxCmd +} From 6e813ab3a82d3834c87a668dfeff26dafae4a3b4 Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Mon, 19 Nov 2018 12:13:45 -0500 Subject: [PATCH 6/9] Change gas & related fields to unsigned integer type (#2839) * Change gas & related fields to unsigned integer type * Implement AddUint64Overflow --- PENDING.md | 3 ++- baseapp/baseapp.go | 10 ++++---- baseapp/baseapp_test.go | 10 ++++---- client/flags.go | 8 +++---- client/lcd/lcd_test.go | 6 ++--- client/utils/rest.go | 2 +- client/utils/utils.go | 10 ++++---- client/utils/utils_test.go | 12 +++++----- cmd/gaia/app/genesis.go | 3 +-- cmd/gaia/cli_test/cli_test.go | 9 ++++--- cmd/gaia/init/genesis_accts_test.go | 14 +++++------ docs/examples/democoin/cmd/democoind/main.go | 1 - server/init.go | 2 -- store/gaskvstore_test.go | 6 ++--- types/context.go | 4 +++- types/gas.go | 25 +++++++++++++++++--- types/gas_test.go | 2 +- types/int.go | 11 +++++++++ types/int_test.go | 25 ++++++++++++++++++++ types/result.go | 4 ++-- x/auth/ante.go | 12 +++++++--- x/auth/ante_test.go | 4 ++-- x/auth/client/txbuilder/txbuilder.go | 4 ++-- x/auth/client/txbuilder/txbuilder_test.go | 4 ++-- x/auth/stdtx.go | 4 ++-- 25 files changed, 127 insertions(+), 68 deletions(-) diff --git a/PENDING.md b/PENDING.md index 1c17731e2..a0985593e 100644 --- a/PENDING.md +++ b/PENDING.md @@ -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 diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 0d5571024..34f9d817d 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -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, } } @@ -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, } } @@ -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) diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 9c2e7e1a0..ba3830e92 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -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) { @@ -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)) @@ -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, } @@ -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{} }) } @@ -813,7 +813,7 @@ func TestTxGasLimits(t *testing.T) { testCases := []struct { tx *txTest - gasUsed int64 + gasUsed uint64 fail bool }{ {newTxCounter(0, 0), 0, false}, diff --git a/client/flags.go b/client/flags.go index dfadab07e..ffb2a0c2f 100644 --- a/client/flags.go +++ b/client/flags.go @@ -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. @@ -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 diff --git a/client/lcd/lcd_test.go b/client/lcd/lcd_test.go index 1be908e98..885e4b914 100644 --- a/client/lcd/lcd_test.go +++ b/client/lcd/lcd_test.go @@ -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, "") @@ -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) { diff --git a/client/utils/rest.go b/client/utils/rest.go index 53c618692..13347098b 100644 --- a/client/utils/rest.go +++ b/client/utils/rest.go @@ -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))) } diff --git a/client/utils/utils.go b/client/utils/utils.go index fee7c7fe2..46bb9799c 100644 --- a/client/utils/utils.go +++ b/client/utils/utils.go @@ -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) @@ -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 @@ -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 diff --git a/client/utils/utils_test.go b/client/utils/utils_test.go index c3bf31569..b22a50806 100644 --- a/client/utils/utils_test.go +++ b/client/utils/utils_test.go @@ -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("") @@ -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}, diff --git a/cmd/gaia/app/genesis.go b/cmd/gaia/app/genesis.go index 7d95bbfa7..5ce9ab8b9 100644 --- a/cmd/gaia/app/genesis.go +++ b/cmd/gaia/app/genesis.go @@ -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) ( @@ -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}, } diff --git a/cmd/gaia/cli_test/cli_test.go b/cmd/gaia/cli_test/cli_test.go index 62f4d0d64..26e6fcaa3 100644 --- a/cmd/gaia/cli_test/cli_test.go +++ b/cmd/gaia/cli_test/cli_test.go @@ -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())) @@ -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())) @@ -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)) diff --git a/cmd/gaia/init/genesis_accts_test.go b/cmd/gaia/init/genesis_accts_test.go index 8825a8cd3..49634a90c 100644 --- a/cmd/gaia/init/genesis_accts_test.go +++ b/cmd/gaia/init/genesis_accts_test.go @@ -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}, } diff --git a/docs/examples/democoin/cmd/democoind/main.go b/docs/examples/democoin/cmd/democoind/main.go index 29e2640fd..730109798 100644 --- a/docs/examples/democoin/cmd/democoind/main.go +++ b/docs/examples/democoin/cmd/democoind/main.go @@ -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) { diff --git a/server/init.go b/server/init.go index 3a6c6adae..e1655c27e 100644 --- a/server/init.go +++ b/server/init.go @@ -15,7 +15,6 @@ import ( tmtypes "github.com/tendermint/tendermint/types" ) - // SimpleGenTx is a simple genesis tx type SimpleGenTx struct { Addr sdk.AccAddress `json:"addr"` @@ -23,7 +22,6 @@ type SimpleGenTx struct { //_____________________________________________________________________ - // Generate a genesis transaction func SimpleAppGenTx(cdc *codec.Codec, pk crypto.PubKey) ( appGenTx, cliPrint json.RawMessage, validator types.GenesisValidator, err error) { diff --git a/store/gaskvstore_test.go b/store/gaskvstore_test.go index ba77bae1f..69f4ae204 100644 --- a/store/gaskvstore_test.go +++ b/store/gaskvstore_test.go @@ -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) { diff --git a/types/context.go b/types/context.go index bfb4c58fe..267e66681 100644 --- a/types/context.go +++ b/types/context.go @@ -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) } diff --git a/types/gas.go b/types/gas.go index 7b0347467..656bd1d04 100644 --- a/types/gas.go +++ b/types/gas.go @@ -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 @@ -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}) } @@ -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 diff --git a/types/gas_test.go b/types/gas_test.go index cd2384d12..f4452053f 100644 --- a/types/gas_test.go +++ b/types/gas_test.go @@ -21,7 +21,7 @@ func TestGasMeter(t *testing.T) { for tcnum, tc := range cases { meter := NewGasMeter(tc.limit) - used := int64(0) + used := uint64(0) for unum, usage := range tc.usage { used += usage diff --git a/types/int.go b/types/int.go index c2bef7a64..2083168e9 100644 --- a/types/int.go +++ b/types/int.go @@ -2,6 +2,7 @@ package types import ( "encoding/json" + "math" "testing" "math/big" @@ -529,6 +530,16 @@ func (i *Uint) UnmarshalJSON(bz []byte) error { //__________________________________________________________________________ +// AddUint64Overflow performs the addition operation on two uint64 integers and +// returns a boolean on whether or not the result overflows. +func AddUint64Overflow(a, b uint64) (uint64, bool) { + if math.MaxUint64-a < b { + return 0, true + } + + return a + b, false +} + // intended to be used with require/assert: require.True(IntEq(...)) func IntEq(t *testing.T, exp, got Int) (*testing.T, bool, string, string, string) { return t, exp.Equal(got), "expected:\t%v\ngot:\t\t%v", exp.String(), got.String() diff --git a/types/int_test.go b/types/int_test.go index cd357c4f7..78bee66bf 100644 --- a/types/int_test.go +++ b/types/int_test.go @@ -590,3 +590,28 @@ func TestEncodingTableUint(t *testing.T) { require.Equal(t, tc.i, i, "Unmarshaled value is different from expected. tc #%d", tcnum) } } + +func TestAddUint64Overflow(t *testing.T) { + testCases := []struct { + a, b uint64 + result uint64 + overflow bool + }{ + {0, 0, 0, false}, + {100, 100, 200, false}, + {math.MaxUint64 / 2, math.MaxUint64/2 + 1, math.MaxUint64, false}, + {math.MaxUint64 / 2, math.MaxUint64/2 + 2, 0, true}, + } + + for i, tc := range testCases { + res, overflow := AddUint64Overflow(tc.a, tc.b) + require.Equal( + t, tc.overflow, overflow, + "invalid overflow result; tc: #%d, a: %d, b: %d", i, tc.a, tc.b, + ) + require.Equal( + t, tc.result, res, + "invalid uint64 result; tc: #%d, a: %d, b: %d", i, tc.a, tc.b, + ) + } +} diff --git a/types/result.go b/types/result.go index 20893a076..924b73b49 100644 --- a/types/result.go +++ b/types/result.go @@ -16,10 +16,10 @@ type Result struct { Log string // GasWanted is the maximum units of work we allow this tx to perform. - GasWanted int64 + GasWanted uint64 // GasUsed is the amount of gas actually consumed. NOTE: unimplemented - GasUsed int64 + GasUsed uint64 // Tx fee amount and denom. FeeAmount int64 diff --git a/x/auth/ante.go b/x/auth/ante.go index 8539be06a..7beb643c5 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -71,6 +71,7 @@ func NewAnteHandler(am AccountKeeper, fck FeeCollectionKeeper) sdk.AnteHandler { if err != nil { return newCtx, err.Result(), true } + // charge gas for the memo newCtx.GasMeter().ConsumeGas(memoCostPerByte*sdk.Gas(len(stdTx.GetMemo())), "memo") @@ -260,13 +261,16 @@ func consumeSignatureVerificationGas(meter sdk.GasMeter, pubkey crypto.PubKey) { } } -func adjustFeesByGas(fees sdk.Coins, gas int64) sdk.Coins { +func adjustFeesByGas(fees sdk.Coins, gas uint64) sdk.Coins { gasCost := gas / gasPerUnitCost gasFees := make(sdk.Coins, len(fees)) + // TODO: Make this not price all coins in the same way + // TODO: Undo int64 casting once unsigned integers are supported for coins for i := 0; i < len(fees); i++ { - gasFees[i] = sdk.NewInt64Coin(fees[i].Denom, gasCost) + gasFees[i] = sdk.NewInt64Coin(fees[i].Denom, int64(gasCost)) } + return fees.Plus(gasFees) } @@ -306,10 +310,12 @@ func ensureSufficientMempoolFees(ctx sdk.Context, stdTx StdTx) sdk.Result { } func setGasMeter(simulate bool, ctx sdk.Context, stdTx StdTx) sdk.Context { - // set the gas meter + // In various cases such as simulation and during the genesis block, we do not + // meter any gas utilization. if simulate || ctx.BlockHeight() == 0 { return ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) } + return ctx.WithGasMeter(sdk.NewGasMeter(stdTx.Fee.Gas)) } diff --git a/x/auth/ante_test.go b/x/auth/ante_test.go index c376d2996..0e8d13957 100644 --- a/x/auth/ante_test.go +++ b/x/auth/ante_test.go @@ -677,7 +677,7 @@ func TestConsumeSignatureVerificationGas(t *testing.T) { tests := []struct { name string args args - gasConsumed int64 + gasConsumed uint64 wantPanic bool }{ {"PubKeyEd25519", args{sdk.NewInfiniteGasMeter(), ed25519.GenPrivKey().PubKey()}, ed25519VerifyCost, false}, @@ -699,7 +699,7 @@ func TestConsumeSignatureVerificationGas(t *testing.T) { func TestAdjustFeesByGas(t *testing.T) { type args struct { fee sdk.Coins - gas int64 + gas uint64 } tests := []struct { name string diff --git a/x/auth/client/txbuilder/txbuilder.go b/x/auth/client/txbuilder/txbuilder.go index f516de451..593d745ab 100644 --- a/x/auth/client/txbuilder/txbuilder.go +++ b/x/auth/client/txbuilder/txbuilder.go @@ -16,7 +16,7 @@ type TxBuilder struct { Codec *codec.Codec AccountNumber int64 Sequence int64 - Gas int64 // TODO: should this turn into uint64? requires further discussion - see #2173 + Gas uint64 GasAdjustment float64 SimulateGas bool ChainID string @@ -61,7 +61,7 @@ func (bldr TxBuilder) WithChainID(chainID string) TxBuilder { } // WithGas returns a copy of the context with an updated gas. -func (bldr TxBuilder) WithGas(gas int64) TxBuilder { +func (bldr TxBuilder) WithGas(gas uint64) TxBuilder { bldr.Gas = gas return bldr } diff --git a/x/auth/client/txbuilder/txbuilder_test.go b/x/auth/client/txbuilder/txbuilder_test.go index 3ad2ad412..f4f9163a0 100644 --- a/x/auth/client/txbuilder/txbuilder_test.go +++ b/x/auth/client/txbuilder/txbuilder_test.go @@ -9,8 +9,8 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth" - "github.com/tendermint/tendermint/crypto/ed25519" stakeTypes "github.com/cosmos/cosmos-sdk/x/stake/types" + "github.com/tendermint/tendermint/crypto/ed25519" ) var ( @@ -23,7 +23,7 @@ func TestTxBuilderBuild(t *testing.T) { Codec *codec.Codec AccountNumber int64 Sequence int64 - Gas int64 + Gas uint64 GasAdjustment float64 SimulateGas bool ChainID string diff --git a/x/auth/stdtx.go b/x/auth/stdtx.go index 84bf88a4b..ba1c65b84 100644 --- a/x/auth/stdtx.go +++ b/x/auth/stdtx.go @@ -70,10 +70,10 @@ func (tx StdTx) GetSignatures() []StdSignature { return tx.Signatures } // which must be above some miminum to be accepted into the mempool. type StdFee struct { Amount sdk.Coins `json:"amount"` - Gas int64 `json:"gas"` + Gas uint64 `json:"gas"` } -func NewStdFee(gas int64, amount ...sdk.Coin) StdFee { +func NewStdFee(gas uint64, amount ...sdk.Coin) StdFee { return StdFee{ Amount: amount, Gas: gas, From 47eed3958b22cf1a842a7167f069469e2c78446b Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Tue, 20 Nov 2018 01:06:14 -0800 Subject: [PATCH 7/9] Clean up Context/MultiStore usage in BaseApp (#2847) --- baseapp/baseapp.go | 62 +++++++++++++++++++++------------------- store/cachemultistore.go | 2 ++ types/context.go | 15 +++++----- 3 files changed, 42 insertions(+), 37 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 34f9d817d..286d58bba 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -200,6 +200,10 @@ func (st *state) CacheMultiStore() sdk.CacheMultiStore { return st.ms.CacheMultiStore() } +func (st *state) Context() sdk.Context { + return st.ctx +} + func (app *BaseApp) setCheckState(header abci.Header) { ms := app.cms.CacheMultiStore() app.checkState = &state{ @@ -383,6 +387,7 @@ func handleQueryCustom(app *BaseApp, path []string, req abci.RequestQuery) (res return sdk.ErrUnknownRequest(fmt.Sprintf("no custom querier found for route %s", path[1])).QueryResult() } + // Cache wrap the commit-multistore for safety. ctx := sdk.NewContext(app.cms.CacheMultiStore(), app.checkState.ctx.BlockHeader(), true, app.Logger). WithMinimumFees(app.minimumFees) @@ -501,14 +506,14 @@ func validateBasicTxMsgs(msgs []sdk.Msg) sdk.Error { return nil } -// retrieve the context for the ante handler and store the tx bytes; store -// the vote infos if the tx runs within the deliverTx() state. -func (app *BaseApp) getContextForAnte(mode runTxMode, txBytes []byte) (ctx sdk.Context) { - ctx = app.getState(mode).ctx.WithTxBytes(txBytes) - if mode == runTxModeDeliver { - ctx = ctx.WithVoteInfos(app.voteInfos) +// retrieve the context for the tx w/ txBytes and other memoized values. +func (app *BaseApp) getContextForTx(mode runTxMode, txBytes []byte) (ctx sdk.Context) { + ctx = app.getState(mode).ctx. + WithTxBytes(txBytes). + WithVoteInfos(app.voteInfos) + if mode == runTxModeSimulate { + ctx, _ = ctx.CacheContext() } - return } @@ -578,21 +583,14 @@ func (app *BaseApp) getState(mode runTxMode) *state { return app.deliverState } -func (app *BaseApp) initializeContext(ctx sdk.Context, mode runTxMode) sdk.Context { - if mode == runTxModeSimulate { - ctx = ctx.WithMultiStore(app.getState(runTxModeSimulate).CacheMultiStore()) - } - return ctx -} +// cacheTxContext returns a new context based off of the provided context with +// a cache wrapped multi-store. +func (app *BaseApp) cacheTxContext(ctx sdk.Context, txBytes []byte) ( + sdk.Context, sdk.CacheMultiStore) { -// cacheTxContext returns a new context based off of the provided context with a -// cache wrapped multi-store and the store itself to allow the caller to write -// changes from the cached multi-store. -func (app *BaseApp) cacheTxContext( - ctx sdk.Context, txBytes []byte, mode runTxMode, -) (sdk.Context, sdk.CacheMultiStore) { - - msCache := app.getState(mode).CacheMultiStore() + ms := ctx.MultiStore() + // TODO: https://github.com/cosmos/cosmos-sdk/issues/2824 + msCache := ms.CacheMultiStore() if msCache.TracingEnabled() { msCache = msCache.WithTracingContext( sdk.TraceContext( @@ -616,8 +614,8 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk // meter so we initialize upfront. var gasWanted uint64 - ctx := app.getContextForAnte(mode, txBytes) - ctx = app.initializeContext(ctx, mode) + ctx := app.getContextForTx(mode, txBytes) + ms := ctx.MultiStore() defer func() { if r := recover(); r != nil { @@ -651,31 +649,37 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk // NOTE: Alternatively, we could require that anteHandler ensures that // writes do not happen if aborted/failed. This may have some // performance benefits, but it'll be more difficult to get right. - anteCtx, msCache = app.cacheTxContext(ctx, txBytes, mode) + anteCtx, msCache = app.cacheTxContext(ctx, txBytes) newCtx, result, abort := app.anteHandler(anteCtx, tx, (mode == runTxModeSimulate)) if abort { return result } if !newCtx.IsZero() { - ctx = newCtx + // At this point, newCtx.MultiStore() is cache wrapped, + // or something else replaced by anteHandler. + // We want the original ms, not one which was cache-wrapped + // for the ante handler. + ctx = newCtx.WithMultiStore(ms) } msCache.Write() gasWanted = result.GasWanted } - if mode == runTxModeSimulate { - result = app.runMsgs(ctx, msgs, mode) - result.GasWanted = gasWanted + if mode == runTxModeCheck { return } // Create a new context based off of the existing context with a cache wrapped // multi-store in case message processing fails. - runMsgCtx, msCache := app.cacheTxContext(ctx, txBytes, mode) + runMsgCtx, msCache := app.cacheTxContext(ctx, txBytes) result = app.runMsgs(runMsgCtx, msgs, mode) result.GasWanted = gasWanted + if mode == runTxModeSimulate { + return + } + // only update state if all messages pass if result.IsOK() { msCache.Write() diff --git a/store/cachemultistore.go b/store/cachemultistore.go index f69ad42aa..ee1977855 100644 --- a/store/cachemultistore.go +++ b/store/cachemultistore.go @@ -11,6 +11,8 @@ import ( // cacheMultiStore holds many cache-wrapped stores. // Implements MultiStore. +// NOTE: a cacheMultiStore (and MultiStores in general) should never expose the +// keys for the substores. type cacheMultiStore struct { db CacheKVStore stores map[StoreKey]CacheWrap diff --git a/types/context.go b/types/context.go index 267e66681..aac05fdfd 100644 --- a/types/context.go +++ b/types/context.go @@ -73,12 +73,12 @@ func (c Context) Value(key interface{}) interface{} { // KVStore fetches a KVStore from the MultiStore. func (c Context) KVStore(key StoreKey) KVStore { - return c.multiStore().GetKVStore(key).Gas(c.GasMeter(), cachedKVGasConfig) + return c.MultiStore().GetKVStore(key).Gas(c.GasMeter(), cachedKVGasConfig) } // TransientStore fetches a TransientStore from the MultiStore. func (c Context) TransientStore(key StoreKey) KVStore { - return c.multiStore().GetKVStore(key).Gas(c.GasMeter(), cachedTransientGasConfig) + return c.MultiStore().GetKVStore(key).Gas(c.GasMeter(), cachedTransientGasConfig) } //---------------------------------------- @@ -143,10 +143,7 @@ const ( contextKeyMinimumFees ) -// NOTE: Do not expose MultiStore. -// MultiStore exposes all the keys. -// Instead, pass the context and the store key. -func (c Context) multiStore() MultiStore { +func (c Context) MultiStore() MultiStore { return c.Value(contextKeyMultiStore).(MultiStore) } @@ -174,7 +171,9 @@ func (c Context) IsCheckTx() bool { return c.Value(contextKeyIsCheckTx).(bool) } func (c Context) MinimumFees() Coins { return c.Value(contextKeyMinimumFees).(Coins) } -func (c Context) WithMultiStore(ms MultiStore) Context { return c.withValue(contextKeyMultiStore, ms) } +func (c Context) WithMultiStore(ms MultiStore) Context { + return c.withValue(contextKeyMultiStore, ms) +} func (c Context) WithBlockHeader(header abci.Header) Context { var _ proto.Message = &header // for cloning. @@ -232,7 +231,7 @@ func (c Context) WithMinimumFees(minFees Coins) Context { // Cache the multistore and return a new cached context. The cached context is // written to the context when writeCache is called. func (c Context) CacheContext() (cc Context, writeCache func()) { - cms := c.multiStore().CacheMultiStore() + cms := c.MultiStore().CacheMultiStore() cc = c.WithMultiStore(cms) return cc, cms.Write } From 41fc538ac7d1d8587e6c983d677ca5f4309b129f Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Tue, 20 Nov 2018 04:22:35 -0500 Subject: [PATCH 8/9] Add Safety Measures to Coin/Coins (#2797) --- PENDING.md | 4 +- cmd/gaia/app/genesis.go | 6 +- cmd/gaia/app/sim_test.go | 2 +- docs/_attic/sdk/core/examples/app2_test.go | 2 +- docs/_attic/sdk/core/examples/app4_test.go | 10 +- docs/examples/democoin/x/cool/app_test.go | 10 +- docs/examples/democoin/x/pow/app_test.go | 6 +- docs/intro/ocap.md | 2 +- types/coin.go | 328 +++++++++++++------- types/coin_benchmark_test.go | 64 ++++ types/coin_test.go | 337 +++++++++------------ types/int.go | 42 ++- types/int_test.go | 25 ++ x/auth/ante.go | 20 +- x/auth/ante_test.go | 1 - x/bank/keeper.go | 6 +- x/bank/msgs_test.go | 8 - x/bank/simulation/msgs.go | 2 +- x/gov/msgs_test.go | 3 - x/slashing/handler_test.go | 6 +- x/slashing/keeper_test.go | 23 +- x/slashing/tick_test.go | 5 +- x/stake/types/msg_test.go | 6 - 23 files changed, 556 insertions(+), 362 deletions(-) create mode 100644 types/coin_benchmark_test.go diff --git a/PENDING.md b/PENDING.md index a0985593e..41e5c39c6 100644 --- a/PENDING.md +++ b/PENDING.md @@ -62,8 +62,10 @@ IMPROVEMENTS * SDK - [x/mock/simulation] [\#2720] major cleanup, introduction of helper objects, reorganization + - \#2821 Codespaces are now strings + - [types] #2776 Improve safety of `Coin` and `Coins` types. Various functions + and methods will panic when a negative amount is discovered. - #2815 Gas unit fields changed from `int64` to `uint64`. - - #2821 Codespaces are now strings * Tendermint - #2796 Update to go-amino 0.14.1 diff --git a/cmd/gaia/app/genesis.go b/cmd/gaia/app/genesis.go index 5ce9ab8b9..ab958e933 100644 --- a/cmd/gaia/app/genesis.go +++ b/cmd/gaia/app/genesis.go @@ -279,10 +279,12 @@ func CollectStdTxs(cdc *codec.Codec, moniker string, genTxsDir string, genDoc tm func NewDefaultGenesisAccount(addr sdk.AccAddress) GenesisAccount { accAuth := auth.NewBaseAccountWithAddress(addr) coins := sdk.Coins{ - {"fooToken", sdk.NewInt(1000)}, - {bondDenom, freeFermionsAcc}, + sdk.NewCoin("fooToken", sdk.NewInt(1000)), + sdk.NewCoin(bondDenom, freeFermionsAcc), } + coins.Sort() + accAuth.Coins = coins return NewGenesisAccount(&accAuth) } diff --git a/cmd/gaia/app/sim_test.go b/cmd/gaia/app/sim_test.go index e56344554..70ae8e12a 100644 --- a/cmd/gaia/app/sim_test.go +++ b/cmd/gaia/app/sim_test.go @@ -63,7 +63,7 @@ func appStateFn(r *rand.Rand, accs []simulation.Account) json.RawMessage { // Randomly generate some genesis accounts for _, acc := range accs { - coins := sdk.Coins{sdk.Coin{stakeTypes.DefaultBondDenom, sdk.NewInt(amount)}} + coins := sdk.Coins{sdk.NewCoin(stakeTypes.DefaultBondDenom, sdk.NewInt(amount))} genesisAccounts = append(genesisAccounts, GenesisAccount{ Address: acc.Address, Coins: coins, diff --git a/docs/_attic/sdk/core/examples/app2_test.go b/docs/_attic/sdk/core/examples/app2_test.go index c59dec264..b7c94bbcf 100644 --- a/docs/_attic/sdk/core/examples/app2_test.go +++ b/docs/_attic/sdk/core/examples/app2_test.go @@ -20,7 +20,7 @@ func TestEncoding(t *testing.T) { sendMsg := MsgSend{ From: addr1, To: addr2, - Amount: sdk.Coins{{"testCoins", sdk.NewInt(100)}}, + Amount: sdk.Coins{sdk.NewCoin("testCoins", sdk.NewInt(100))}, } // Construct transaction diff --git a/docs/_attic/sdk/core/examples/app4_test.go b/docs/_attic/sdk/core/examples/app4_test.go index 7d95d7b57..86be8ea12 100644 --- a/docs/_attic/sdk/core/examples/app4_test.go +++ b/docs/_attic/sdk/core/examples/app4_test.go @@ -30,7 +30,7 @@ func InitTestChain(bc *bapp.BaseApp, chainID string, addrs ...sdk.AccAddress) { for _, addr := range addrs { acc := GenesisAccount{ Address: addr, - Coins: sdk.Coins{{"testCoin", sdk.NewInt(100)}}, + Coins: sdk.Coins{sdk.NewCoin("testCoin", sdk.NewInt(100))}, } accounts = append(accounts, &acc) } @@ -61,12 +61,12 @@ func TestBadMsg(t *testing.T) { addr2 := priv2.PubKey().Address().Bytes() // Attempt to spend non-existent funds - msg := GenerateSpendMsg(addr1, addr2, sdk.Coins{{"testCoin", sdk.NewInt(100)}}) + msg := GenerateSpendMsg(addr1, addr2, sdk.Coins{sdk.NewCoin("testCoin", sdk.NewInt(100))}) // Construct transaction fee := auth.StdFee{ Gas: 1000000000000000, - Amount: sdk.Coins{{"testCoin", sdk.NewInt(0)}}, + Amount: sdk.Coins{sdk.NewCoin("testCoin", sdk.NewInt(0))}, } signBytes := auth.StdSignBytes("test-chain", 0, 0, fee, []sdk.Msg{msg}, "") sig, err := priv1.Sign(signBytes) @@ -108,11 +108,11 @@ func TestMsgSend(t *testing.T) { InitTestChain(bc, "test-chain", addr1) // Send funds to addr2 - msg := GenerateSpendMsg(addr1, addr2, sdk.Coins{{"testCoin", sdk.NewInt(100)}}) + msg := GenerateSpendMsg(addr1, addr2, sdk.Coins{sdk.NewCoin("testCoin", sdk.NewInt(100))}) fee := auth.StdFee{ Gas: 1000000000000000, - Amount: sdk.Coins{{"testCoin", sdk.NewInt(0)}}, + Amount: sdk.Coins{sdk.NewCoin("testCoin", sdk.NewInt(0))}, } signBytes := auth.StdSignBytes("test-chain", 0, 0, fee, []sdk.Msg{msg}, "") sig, err := priv1.Sign(signBytes) diff --git a/docs/examples/democoin/x/cool/app_test.go b/docs/examples/democoin/x/cool/app_test.go index 3f1dd6734..7bfc8b9cc 100644 --- a/docs/examples/democoin/x/cool/app_test.go +++ b/docs/examples/democoin/x/cool/app_test.go @@ -90,15 +90,15 @@ func TestMsgQuiz(t *testing.T) { // Set the trend, submit a really cool quiz and check for reward mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{setTrendMsg1}, []int64{0}, []int64{0}, true, true, priv1) mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{quizMsg1}, []int64{0}, []int64{1}, true, true, priv1) - mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"icecold", sdk.NewInt(69)}}) + mock.CheckBalance(t, mapp, addr1, sdk.Coins{sdk.NewCoin("icecold", sdk.NewInt(69))}) mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{quizMsg2}, []int64{0}, []int64{2}, false, false, priv1) // result without reward - mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"icecold", sdk.NewInt(69)}}) + mock.CheckBalance(t, mapp, addr1, sdk.Coins{sdk.NewCoin("icecold", sdk.NewInt(69))}) mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{quizMsg1}, []int64{0}, []int64{3}, true, true, priv1) - mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"icecold", sdk.NewInt(138)}}) + mock.CheckBalance(t, mapp, addr1, sdk.Coins{sdk.NewCoin("icecold", sdk.NewInt(138))}) mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{setTrendMsg2}, []int64{0}, []int64{4}, true, true, priv1) // reset the trend mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{quizMsg1}, []int64{0}, []int64{5}, false, false, priv1) // the same answer will nolonger do! - mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"icecold", sdk.NewInt(138)}}) + mock.CheckBalance(t, mapp, addr1, sdk.Coins{sdk.NewCoin("icecold", sdk.NewInt(138))}) mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{quizMsg2}, []int64{0}, []int64{6}, true, true, priv1) // earlier answer now relevant again - mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"badvibesonly", sdk.NewInt(69)}, {"icecold", sdk.NewInt(138)}}) + mock.CheckBalance(t, mapp, addr1, sdk.Coins{sdk.NewCoin("badvibesonly", sdk.NewInt(69)), sdk.NewCoin("icecold", sdk.NewInt(138))}) mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{setTrendMsg3}, []int64{0}, []int64{7}, false, false, priv1) // expect to fail to set the trend to something which is not cool } diff --git a/docs/examples/democoin/x/pow/app_test.go b/docs/examples/democoin/x/pow/app_test.go index 41901f097..58a7d3538 100644 --- a/docs/examples/democoin/x/pow/app_test.go +++ b/docs/examples/democoin/x/pow/app_test.go @@ -75,12 +75,12 @@ func TestMsgMine(t *testing.T) { // Mine and check for reward mineMsg1 := GenerateMsgMine(addr1, 1, 2) mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{mineMsg1}, []int64{0}, []int64{0}, true, true, priv1) - mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"pow", sdk.NewInt(1)}}) + mock.CheckBalance(t, mapp, addr1, sdk.Coins{sdk.NewCoin("pow", sdk.NewInt(1))}) // Mine again and check for reward mineMsg2 := GenerateMsgMine(addr1, 2, 3) mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{mineMsg2}, []int64{0}, []int64{1}, true, true, priv1) - mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"pow", sdk.NewInt(2)}}) + mock.CheckBalance(t, mapp, addr1, sdk.Coins{sdk.NewCoin("pow", sdk.NewInt(2))}) // Mine again - should be invalid mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{mineMsg2}, []int64{0}, []int64{1}, false, false, priv1) - mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"pow", sdk.NewInt(2)}}) + mock.CheckBalance(t, mapp, addr1, sdk.Coins{sdk.NewCoin("pow", sdk.NewInt(2))}) } diff --git a/docs/intro/ocap.md b/docs/intro/ocap.md index 7d57d0c34..cea7c108d 100644 --- a/docs/intro/ocap.md +++ b/docs/intro/ocap.md @@ -63,7 +63,7 @@ principle: type AppAccount struct {...} var account := &AppAccount{ Address: pub.Address(), - Coins: sdk.Coins{{"ATM", 100}}, + Coins: sdk.Coins{sdk.NewInt64Coin("ATM", 100)}, } var sumValue := externalModule.ComputeSumValue(account) ``` diff --git a/types/coin.go b/types/coin.go index 31f0e98a4..5f9020e84 100644 --- a/types/coin.go +++ b/types/coin.go @@ -4,23 +4,41 @@ import ( "fmt" "regexp" "sort" - "strconv" "strings" ) -// Coin hold some amount of one currency +//----------------------------------------------------------------------------- +// Coin + +// Coin hold some amount of one currency. +// +// CONTRACT: A coin will never hold a negative amount of any denomination. +// +// TODO: Make field members private for further safety. type Coin struct { - Denom string `json:"denom"` - Amount Int `json:"amount"` + Denom string `json:"denom"` + + // To allow the use of unsigned integers (see: #1273) a larger refactor will + // need to be made. So we use signed integers for now with safety measures in + // place preventing negative values being used. + Amount Int `json:"amount"` } +// NewCoin returns a new coin with a denomination and amount. It will panic if +// the amount is negative. func NewCoin(denom string, amount Int) Coin { + if amount.LT(ZeroInt()) { + panic("negative coin amount") + } + return Coin{ Denom: denom, Amount: amount, } } +// NewInt64Coin returns a new coin with a denomination and amount. It will panic +// if the amount is negative. func NewInt64Coin(denom string, amount int64) Coin { return NewCoin(denom, NewInt(amount)) } @@ -57,33 +75,46 @@ func (coin Coin) IsEqual(other Coin) bool { return coin.SameDenomAs(other) && (coin.Amount.Equal(other.Amount)) } -// IsPositive returns true if coin amount is positive +// Adds amounts of two coins with same denom. If the coins differ in denom then +// it panics. +func (coin Coin) Plus(coinB Coin) Coin { + if !coin.SameDenomAs(coinB) { + panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, coinB.Denom)) + } + + return Coin{coin.Denom, coin.Amount.Add(coinB.Amount)} +} + +// Subtracts amounts of two coins with same denom. If the coins differ in denom +// then it panics. +func (coin Coin) Minus(coinB Coin) Coin { + if !coin.SameDenomAs(coinB) { + panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, coinB.Denom)) + } + + res := Coin{coin.Denom, coin.Amount.Sub(coinB.Amount)} + if !res.IsNotNegative() { + panic("negative count amount") + } + + return res +} + +// IsPositive returns true if coin amount is positive. +// +// TODO: Remove once unsigned integers are used. func (coin Coin) IsPositive() bool { return (coin.Amount.Sign() == 1) } -// IsNotNegative returns true if coin amount is not negative +// IsNotNegative returns true if coin amount is not negative and false otherwise. +// +// TODO: Remove once unsigned integers are used. func (coin Coin) IsNotNegative() bool { return (coin.Amount.Sign() != -1) } -// Adds amounts of two coins with same denom -func (coin Coin) Plus(coinB Coin) Coin { - if !coin.SameDenomAs(coinB) { - return coin - } - return Coin{coin.Denom, coin.Amount.Add(coinB.Amount)} -} - -// Subtracts amounts of two coins with same denom -func (coin Coin) Minus(coinB Coin) Coin { - if !coin.SameDenomAs(coinB) { - return coin - } - return Coin{coin.Denom, coin.Amount.Sub(coinB.Amount)} -} - -//---------------------------------------- +//----------------------------------------------------------------------------- // Coins // Coins is a set of Coin, one per currency @@ -101,127 +132,157 @@ func (coins Coins) String() string { return out[:len(out)-1] } -// IsValid asserts the Coins are sorted, and don't have 0 amounts +// IsValid asserts the Coins are sorted and have positive amounts. func (coins Coins) IsValid() bool { switch len(coins) { case 0: return true case 1: - return !coins[0].IsZero() + return coins[0].IsPositive() default: lowDenom := coins[0].Denom + for _, coin := range coins[1:] { if coin.Denom <= lowDenom { return false } - if coin.IsZero() { + if !coin.IsPositive() { return false } + // we compare each coin against the last denom lowDenom = coin.Denom } + return true } } -// Plus combines two sets of coins -// CONTRACT: Plus will never return Coins where one Coin has a 0 amount. +// Plus adds two sets of coins. +// +// e.g. +// {2A} + {A, 2B} = {3A, 2B} +// {2A} + {0B} = {2A} +// +// NOTE: Plus operates under the invariant that coins are sorted by +// denominations. +// +// CONTRACT: Plus will never return Coins where one Coin has a non-positive +// amount. In otherwords, IsValid will always return true. func (coins Coins) Plus(coinsB Coins) Coins { + return coins.safePlus(coinsB) +} + +// safePlus will perform addition of two coins sets. If both coin sets are +// empty, then an empty set is returned. If only a single set is empty, the +// other set is returned. Otherwise, the coins are compared in order of their +// denomination and addition only occurs when the denominations match, otherwise +// the coin is simply added to the sum assuming it's not zero. +func (coins Coins) safePlus(coinsB Coins) Coins { sum := ([]Coin)(nil) indexA, indexB := 0, 0 lenA, lenB := len(coins), len(coinsB) + for { if indexA == lenA { if indexB == lenB { + // return nil coins if both sets are empty return sum } - return append(sum, coinsB[indexB:]...) + + // return set B (excluding zero coins) if set A is empty + return append(sum, removeZeroCoins(coinsB[indexB:])...) } else if indexB == lenB { - return append(sum, coins[indexA:]...) + // return set A (excluding zero coins) if set B is empty + return append(sum, removeZeroCoins(coins[indexA:])...) } + coinA, coinB := coins[indexA], coinsB[indexB] + switch strings.Compare(coinA.Denom, coinB.Denom) { - case -1: - if coinA.IsZero() { - // ignore 0 sum coin type - } else { + case -1: // coin A denom < coin B denom + if !coinA.IsZero() { sum = append(sum, coinA) } + indexA++ - case 0: - if coinA.Amount.Add(coinB.Amount).IsZero() { - // ignore 0 sum coin type - } else { - sum = append(sum, coinA.Plus(coinB)) + + case 0: // coin A denom == coin B denom + res := coinA.Plus(coinB) + if !res.IsZero() { + sum = append(sum, res) } + indexA++ indexB++ - case 1: - if coinB.IsZero() { - // ignore 0 sum coin type - } else { + + case 1: // coin A denom > coin B denom + if !coinB.IsZero() { sum = append(sum, coinB) } + indexB++ } } } -// Negative returns a set of coins with all amount negative -func (coins Coins) Negative() Coins { - res := make([]Coin, 0, len(coins)) - for _, coin := range coins { - res = append(res, Coin{ - Denom: coin.Denom, - Amount: coin.Amount.Neg(), - }) - } - return res -} - -// Minus subtracts a set of coins from another (adds the inverse) +// Minus subtracts a set of coins from another. +// +// e.g. +// {2A, 3B} - {A} = {A, 3B} +// {2A} - {0B} = {2A} +// {A, B} - {A} = {B} +// +// CONTRACT: Minus will never return Coins where one Coin has a non-positive +// amount. In otherwords, IsValid will always return true. func (coins Coins) Minus(coinsB Coins) Coins { - return coins.Plus(coinsB.Negative()) + diff, hasNeg := coins.SafeMinus(coinsB) + if hasNeg { + panic("negative coin amount") + } + + return diff } -// IsAllGT returns True iff for every denom in coins, the denom is present at a +// SafeMinus performs the same arithmetic as Minus but returns a boolean if any +// negative coin amount was returned. +func (coins Coins) SafeMinus(coinsB Coins) (Coins, bool) { + diff := coins.safePlus(coinsB.negative()) + return diff, !diff.IsNotNegative() +} + +// IsAllGT returns true iff for every denom in coins, the denom is present at a // greater amount in coinsB. func (coins Coins) IsAllGT(coinsB Coins) bool { - diff := coins.Minus(coinsB) + diff, _ := coins.SafeMinus(coinsB) if len(diff) == 0 { return false } + return diff.IsPositive() } -// IsAllGTE returns True iff for every denom in coins, the denom is present at an -// equal or greater amount in coinsB. +// IsAllGTE returns true iff for every denom in coins, the denom is present at +// an equal or greater amount in coinsB. func (coins Coins) IsAllGTE(coinsB Coins) bool { - diff := coins.Minus(coinsB) + diff, _ := coins.SafeMinus(coinsB) if len(diff) == 0 { return true } + return diff.IsNotNegative() } // IsAllLT returns True iff for every denom in coins, the denom is present at // a smaller amount in coinsB. func (coins Coins) IsAllLT(coinsB Coins) bool { - diff := coinsB.Minus(coins) - if len(diff) == 0 { - return false - } - return diff.IsPositive() + return coinsB.IsAllGT(coins) } -// IsAllLTE returns True iff for every denom in coins, the denom is present at +// IsAllLTE returns true iff for every denom in coins, the denom is present at // a smaller or equal amount in coinsB. func (coins Coins) IsAllLTE(coinsB Coins) bool { - diff := coinsB.Minus(coins) - if len(diff) == 0 { - return true - } - return diff.IsNotNegative() + return coinsB.IsAllGTE(coins) } // IsZero returns true if there are no coins or all coins are zero. @@ -239,40 +300,22 @@ func (coins Coins) IsEqual(coinsB Coins) bool { if len(coins) != len(coinsB) { return false } + + coins = coins.Sort() + coinsB = coinsB.Sort() + for i := 0; i < len(coins); i++ { if coins[i].Denom != coinsB[i].Denom || !coins[i].Amount.Equal(coinsB[i].Amount) { return false } } + return true } -// IsPositive returns true if there is at least one coin, and all -// currencies have a positive value -func (coins Coins) IsPositive() bool { - if len(coins) == 0 { - return false - } - for _, coin := range coins { - if !coin.IsPositive() { - return false - } - } - return true -} - -// IsNotNegative returns true if there is no currency with a negative value -// (even no coins is true here) -func (coins Coins) IsNotNegative() bool { - if len(coins) == 0 { - return true - } - for _, coin := range coins { - if !coin.IsNotNegative() { - return false - } - } - return true +// Empty returns true if there are no coins and false otherwise. +func (coins Coins) Empty() bool { + return len(coins) == 0 } // Returns the amount of a denom from coins @@ -280,15 +323,18 @@ func (coins Coins) AmountOf(denom string) Int { switch len(coins) { case 0: return ZeroInt() + case 1: coin := coins[0] if coin.Denom == denom { return coin.Amount } return ZeroInt() + default: midIdx := len(coins) / 2 // 2:1, 3:1, 4:2 coin := coins[midIdx] + if denom < coin.Denom { return coins[:midIdx].AmountOf(denom) } else if denom == coin.Denom { @@ -299,7 +345,75 @@ func (coins Coins) AmountOf(denom string) Int { } } -//---------------------------------------- +// IsPositive returns true if there is at least one coin and all currencies +// have a positive value. +// +// TODO: Remove once unsigned integers are used. +func (coins Coins) IsPositive() bool { + if len(coins) == 0 { + return false + } + + for _, coin := range coins { + if !coin.IsPositive() { + return false + } + } + + return true +} + +// IsNotNegative returns true if there is no coin amount with a negative value +// (even no coins is true here). +// +// TODO: Remove once unsigned integers are used. +func (coins Coins) IsNotNegative() bool { + if len(coins) == 0 { + return true + } + + for _, coin := range coins { + if !coin.IsNotNegative() { + return false + } + } + + return true +} + +// negative returns a set of coins with all amount negative. +// +// TODO: Remove once unsigned integers are used. +func (coins Coins) negative() Coins { + res := make([]Coin, 0, len(coins)) + + for _, coin := range coins { + res = append(res, Coin{ + Denom: coin.Denom, + Amount: coin.Amount.Neg(), + }) + } + + return res +} + +// removeZeroCoins removes all zero coins from the given coin set in-place. +func removeZeroCoins(coins Coins) Coins { + i, l := 0, len(coins) + for i < l { + if coins[i].IsZero() { + // remove coin + coins = append(coins[:i], coins[i+1:]...) + l-- + } else { + i++ + } + } + + return coins[:i] +} + +//----------------------------------------------------------------------------- // Sort interface //nolint @@ -315,7 +429,7 @@ func (coins Coins) Sort() Coins { return coins } -//---------------------------------------- +//----------------------------------------------------------------------------- // Parsing var ( @@ -333,17 +447,17 @@ func ParseCoin(coinStr string) (coin Coin, err error) { matches := reCoin.FindStringSubmatch(coinStr) if matches == nil { - err = fmt.Errorf("invalid coin expression: %s", coinStr) - return + return Coin{}, fmt.Errorf("invalid coin expression: %s", coinStr) } + denomStr, amountStr := matches[2], matches[1] - amount, err := strconv.Atoi(amountStr) - if err != nil { - return + amount, ok := NewIntFromString(amountStr) + if !ok { + return Coin{}, fmt.Errorf("failed to parse coin amount: %s", amountStr) } - return Coin{denomStr, NewInt(int64(amount))}, nil + return Coin{denomStr, amount}, nil } // ParseCoins will parse out a list of coins separated by commas. diff --git a/types/coin_benchmark_test.go b/types/coin_benchmark_test.go new file mode 100644 index 000000000..9c13bf772 --- /dev/null +++ b/types/coin_benchmark_test.go @@ -0,0 +1,64 @@ +package types + +import ( + "fmt" + "testing" +) + +func BenchmarkCoinsAdditionIntersect(b *testing.B) { + benchmarkingFunc := func(numCoinsA int, numCoinsB int) func(b *testing.B) { + return func(b *testing.B) { + coinsA := Coins(make([]Coin, numCoinsA)) + coinsB := Coins(make([]Coin, numCoinsB)) + + for i := 0; i < numCoinsA; i++ { + coinsA[i] = NewCoin("COINZ_"+string(i), NewInt(int64(i))) + } + for i := 0; i < numCoinsB; i++ { + coinsB[i] = NewCoin("COINZ_"+string(i), NewInt(int64(i))) + } + + b.ResetTimer() + + for i := 0; i < b.N; i++ { + coinsA.Plus(coinsB) + } + } + } + + benchmarkSizes := [][]int{{1, 1}, {5, 5}, {5, 20}, {1, 1000}, {2, 1000}} + for i := 0; i < len(benchmarkSizes); i++ { + sizeA := benchmarkSizes[i][0] + sizeB := benchmarkSizes[i][1] + b.Run(fmt.Sprintf("sizes: A_%d, B_%d", sizeA, sizeB), benchmarkingFunc(sizeA, sizeB)) + } +} + +func BenchmarkCoinsAdditionNoIntersect(b *testing.B) { + benchmarkingFunc := func(numCoinsA int, numCoinsB int) func(b *testing.B) { + return func(b *testing.B) { + coinsA := Coins(make([]Coin, numCoinsA)) + coinsB := Coins(make([]Coin, numCoinsB)) + + for i := 0; i < numCoinsA; i++ { + coinsA[i] = NewCoin("COINZ_"+string(numCoinsB+i), NewInt(int64(i))) + } + for i := 0; i < numCoinsB; i++ { + coinsB[i] = NewCoin("COINZ_"+string(i), NewInt(int64(i))) + } + + b.ResetTimer() + + for i := 0; i < b.N; i++ { + coinsA.Plus(coinsB) + } + } + } + + benchmarkSizes := [][]int{{1, 1}, {5, 5}, {5, 20}, {1, 1000}, {2, 1000}, {1000, 2}} + for i := 0; i < len(benchmarkSizes); i++ { + sizeA := benchmarkSizes[i][0] + sizeB := benchmarkSizes[i][1] + b.Run(fmt.Sprintf("sizes: A_%d, B_%d", sizeA, sizeB), benchmarkingFunc(sizeA, sizeB)) + } +} diff --git a/types/coin_test.go b/types/coin_test.go index 77307f22a..774534860 100644 --- a/types/coin_test.go +++ b/types/coin_test.go @@ -1,43 +1,20 @@ package types import ( - "fmt" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestIsPositiveCoin(t *testing.T) { - cases := []struct { - inputOne Coin - expected bool - }{ - {NewInt64Coin("A", 1), true}, - {NewInt64Coin("A", 0), false}, - {NewInt64Coin("a", -1), false}, - } +// ---------------------------------------------------------------------------- +// Coin tests - for tcIndex, tc := range cases { - res := tc.inputOne.IsPositive() - require.Equal(t, tc.expected, res, "%s positivity is incorrect, tc #%d", tc.inputOne.String(), tcIndex) - } -} - -func TestIsNotNegativeCoin(t *testing.T) { - cases := []struct { - inputOne Coin - expected bool - }{ - {NewInt64Coin("A", 1), true}, - {NewInt64Coin("A", 0), true}, - {NewInt64Coin("a", -1), false}, - } - - for tcIndex, tc := range cases { - res := tc.inputOne.IsNotNegative() - require.Equal(t, tc.expected, res, "%s not-negativity is incorrect, tc #%d", tc.inputOne.String(), tcIndex) - } +func TestCoin(t *testing.T) { + require.Panics(t, func() { NewInt64Coin("A", -1) }) + require.Panics(t, func() { NewCoin("A", NewInt(-1)) }) + require.Equal(t, NewInt(5), NewInt64Coin("A", 5).Amount) + require.Equal(t, NewInt(5), NewCoin("A", NewInt(5)).Amount) } func TestSameDenomAsCoin(t *testing.T) { @@ -49,8 +26,7 @@ func TestSameDenomAsCoin(t *testing.T) { {NewInt64Coin("A", 1), NewInt64Coin("A", 1), true}, {NewInt64Coin("A", 1), NewInt64Coin("a", 1), false}, {NewInt64Coin("a", 1), NewInt64Coin("b", 1), false}, - {NewInt64Coin("stake", 1), NewInt64Coin("stake", 10), true}, - {NewInt64Coin("stake", -11), NewInt64Coin("stake", 10), true}, + {NewInt64Coin("steak", 1), NewInt64Coin("steak", 10), true}, } for tcIndex, tc := range cases { @@ -59,6 +35,78 @@ func TestSameDenomAsCoin(t *testing.T) { } } +func TestIsEqualCoin(t *testing.T) { + cases := []struct { + inputOne Coin + inputTwo Coin + expected bool + }{ + {NewInt64Coin("A", 1), NewInt64Coin("A", 1), true}, + {NewInt64Coin("A", 1), NewInt64Coin("a", 1), false}, + {NewInt64Coin("a", 1), NewInt64Coin("b", 1), false}, + {NewInt64Coin("steak", 1), NewInt64Coin("steak", 10), false}, + } + + for tcIndex, tc := range cases { + res := tc.inputOne.IsEqual(tc.inputTwo) + require.Equal(t, tc.expected, res, "coin equality relation is incorrect, tc #%d", tcIndex) + } +} + +func TestPlusCoin(t *testing.T) { + cases := []struct { + inputOne Coin + inputTwo Coin + expected Coin + shouldPanic bool + }{ + {NewInt64Coin("A", 1), NewInt64Coin("A", 1), NewInt64Coin("A", 2), false}, + {NewInt64Coin("A", 1), NewInt64Coin("A", 0), NewInt64Coin("A", 1), false}, + {NewInt64Coin("A", 1), NewInt64Coin("B", 1), NewInt64Coin("A", 1), true}, + } + + for tcIndex, tc := range cases { + if tc.shouldPanic { + require.Panics(t, func() { tc.inputOne.Plus(tc.inputTwo) }) + } else { + res := tc.inputOne.Plus(tc.inputTwo) + require.Equal(t, tc.expected, res, "sum of coins is incorrect, tc #%d", tcIndex) + } + } +} + +func TestMinusCoin(t *testing.T) { + cases := []struct { + inputOne Coin + inputTwo Coin + expected Coin + shouldPanic bool + }{ + {NewInt64Coin("A", 1), NewInt64Coin("B", 1), NewInt64Coin("A", 1), true}, + {NewInt64Coin("A", 10), NewInt64Coin("A", 1), NewInt64Coin("A", 9), false}, + {NewInt64Coin("A", 5), NewInt64Coin("A", 3), NewInt64Coin("A", 2), false}, + {NewInt64Coin("A", 5), NewInt64Coin("A", 0), NewInt64Coin("A", 5), false}, + {NewInt64Coin("A", 1), NewInt64Coin("A", 5), Coin{}, true}, + } + + for tcIndex, tc := range cases { + if tc.shouldPanic { + require.Panics(t, func() { tc.inputOne.Minus(tc.inputTwo) }) + } else { + res := tc.inputOne.Minus(tc.inputTwo) + require.Equal(t, tc.expected, res, "difference of coins is incorrect, tc #%d", tcIndex) + } + } + + tc := struct { + inputOne Coin + inputTwo Coin + expected int64 + }{NewInt64Coin("A", 1), NewInt64Coin("A", 1), 0} + res := tc.inputOne.Minus(tc.inputTwo) + require.Equal(t, tc.expected, res.Amount.Int64()) +} + func TestIsGTECoin(t *testing.T) { cases := []struct { inputOne Coin @@ -67,8 +115,7 @@ func TestIsGTECoin(t *testing.T) { }{ {NewInt64Coin("A", 1), NewInt64Coin("A", 1), true}, {NewInt64Coin("A", 2), NewInt64Coin("A", 1), true}, - {NewInt64Coin("A", -1), NewInt64Coin("A", 5), false}, - {NewInt64Coin("a", 1), NewInt64Coin("b", 1), false}, + {NewInt64Coin("A", 1), NewInt64Coin("B", 1), false}, } for tcIndex, tc := range cases { @@ -85,7 +132,6 @@ func TestIsLTCoin(t *testing.T) { }{ {NewInt64Coin("A", 1), NewInt64Coin("A", 1), false}, {NewInt64Coin("A", 2), NewInt64Coin("A", 1), false}, - {NewInt64Coin("A", -1), NewInt64Coin("A", 5), true}, {NewInt64Coin("a", 0), NewInt64Coin("b", 1), false}, {NewInt64Coin("a", 1), NewInt64Coin("b", 1), false}, {NewInt64Coin("a", 1), NewInt64Coin("a", 1), false}, @@ -98,76 +144,18 @@ func TestIsLTCoin(t *testing.T) { } } -func TestIsEqualCoin(t *testing.T) { - cases := []struct { - inputOne Coin - inputTwo Coin - expected bool - }{ - {NewInt64Coin("A", 1), NewInt64Coin("A", 1), true}, - {NewInt64Coin("A", 1), NewInt64Coin("a", 1), false}, - {NewInt64Coin("a", 1), NewInt64Coin("b", 1), false}, - {NewInt64Coin("stake", 1), NewInt64Coin("stake", 10), false}, - {NewInt64Coin("stake", -11), NewInt64Coin("stake", 10), false}, - } +func TestCoinIsZero(t *testing.T) { + coin := NewInt64Coin("A", 0) + res := coin.IsZero() + require.True(t, res) - for tcIndex, tc := range cases { - res := tc.inputOne.IsEqual(tc.inputTwo) - require.Equal(t, tc.expected, res, "coin equality relation is incorrect, tc #%d", tcIndex) - } + coin = NewInt64Coin("A", 1) + res = coin.IsZero() + require.False(t, res) } -func TestPlusCoin(t *testing.T) { - cases := []struct { - inputOne Coin - inputTwo Coin - expected Coin - }{ - {NewInt64Coin("A", 1), NewInt64Coin("A", 1), NewInt64Coin("A", 2)}, - {NewInt64Coin("A", 1), NewInt64Coin("B", 1), NewInt64Coin("A", 1)}, - {NewInt64Coin("asdf", -4), NewInt64Coin("asdf", 5), NewInt64Coin("asdf", 1)}, - } - - for tcIndex, tc := range cases { - res := tc.inputOne.Plus(tc.inputTwo) - require.Equal(t, tc.expected, res, "sum of coins is incorrect, tc #%d", tcIndex) - } - - tc := struct { - inputOne Coin - inputTwo Coin - expected int64 - }{NewInt64Coin("asdf", -1), NewInt64Coin("asdf", 1), 0} - res := tc.inputOne.Plus(tc.inputTwo) - require.Equal(t, tc.expected, res.Amount.Int64()) -} - -func TestMinusCoin(t *testing.T) { - cases := []struct { - inputOne Coin - inputTwo Coin - expected Coin - }{ - - {NewInt64Coin("A", 1), NewInt64Coin("B", 1), NewInt64Coin("A", 1)}, - {NewInt64Coin("asdf", -4), NewInt64Coin("asdf", 5), NewInt64Coin("asdf", -9)}, - {NewInt64Coin("asdf", 10), NewInt64Coin("asdf", 1), NewInt64Coin("asdf", 9)}, - } - - for tcIndex, tc := range cases { - res := tc.inputOne.Minus(tc.inputTwo) - require.Equal(t, tc.expected, res, "difference of coins is incorrect, tc #%d", tcIndex) - } - - tc := struct { - inputOne Coin - inputTwo Coin - expected int64 - }{NewInt64Coin("A", 1), NewInt64Coin("A", 1), 0} - res := tc.inputOne.Minus(tc.inputTwo) - require.Equal(t, tc.expected, res.Amount.Int64()) - -} +// ---------------------------------------------------------------------------- +// Coins tests func TestIsZeroCoins(t *testing.T) { cases := []struct { @@ -199,8 +187,7 @@ func TestEqualCoins(t *testing.T) { {Coins{NewInt64Coin("A", 0)}, Coins{NewInt64Coin("B", 0)}, false}, {Coins{NewInt64Coin("A", 0)}, Coins{NewInt64Coin("A", 1)}, false}, {Coins{NewInt64Coin("A", 0)}, Coins{NewInt64Coin("A", 0), NewInt64Coin("B", 1)}, false}, - // TODO: is it expected behaviour? shouldn't we sort the coins before comparing them? - {Coins{NewInt64Coin("A", 0), NewInt64Coin("B", 1)}, Coins{NewInt64Coin("B", 1), NewInt64Coin("A", 0)}, false}, + {Coins{NewInt64Coin("A", 0), NewInt64Coin("B", 1)}, Coins{NewInt64Coin("B", 1), NewInt64Coin("A", 0)}, true}, } for tcnum, tc := range cases { @@ -209,16 +196,65 @@ func TestEqualCoins(t *testing.T) { } } -func TestCoins(t *testing.T) { +func TestPlusCoins(t *testing.T) { + zero := NewInt(0) + one := NewInt(1) + two := NewInt(2) - //Define the coins to be used in tests + cases := []struct { + inputOne Coins + inputTwo Coins + expected Coins + }{ + {Coins{{"A", one}, {"B", one}}, Coins{{"A", one}, {"B", one}}, Coins{{"A", two}, {"B", two}}}, + {Coins{{"A", zero}, {"B", one}}, Coins{{"A", zero}, {"B", zero}}, Coins{{"B", one}}}, + {Coins{{"A", two}}, Coins{{"B", zero}}, Coins{{"A", two}}}, + {Coins{{"A", one}}, Coins{{"A", one}, {"B", two}}, Coins{{"A", two}, {"B", two}}}, + {Coins{{"A", zero}, {"B", zero}}, Coins{{"A", zero}, {"B", zero}}, Coins(nil)}, + } + + for tcIndex, tc := range cases { + res := tc.inputOne.Plus(tc.inputTwo) + assert.True(t, res.IsValid()) + require.Equal(t, tc.expected, res, "sum of coins is incorrect, tc #%d", tcIndex) + } +} + +func TestMinusCoins(t *testing.T) { + zero := NewInt(0) + one := NewInt(1) + two := NewInt(2) + + testCases := []struct { + inputOne Coins + inputTwo Coins + expected Coins + shouldPanic bool + }{ + {Coins{{"A", two}}, Coins{{"A", one}, {"B", two}}, Coins{{"A", one}, {"B", two}}, true}, + {Coins{{"A", two}}, Coins{{"B", zero}}, Coins{{"A", two}}, false}, + {Coins{{"A", one}}, Coins{{"B", zero}}, Coins{{"A", one}}, false}, + {Coins{{"A", one}, {"B", one}}, Coins{{"A", one}}, Coins{{"B", one}}, false}, + {Coins{{"A", one}, {"B", one}}, Coins{{"A", two}}, Coins{}, true}, + } + + for i, tc := range testCases { + if tc.shouldPanic { + require.Panics(t, func() { tc.inputOne.Minus(tc.inputTwo) }) + } else { + res := tc.inputOne.Minus(tc.inputTwo) + assert.True(t, res.IsValid()) + require.Equal(t, tc.expected, res, "sum of coins is incorrect, tc #%d", i) + } + } +} + +func TestCoins(t *testing.T) { good := Coins{ {"GAS", NewInt(1)}, {"MINERAL", NewInt(1)}, {"TREE", NewInt(1)}, } - neg := good.Negative() - sum := good.Plus(neg) empty := Coins{ {"GOLD", NewInt(0)}, } @@ -228,6 +264,7 @@ func TestCoins(t *testing.T) { {"GAS", NewInt(1)}, {"MINERAL", NewInt(1)}, } + // both are after the first one, but the second and third are in the wrong order badSort2 := Coins{ {"GAS", NewInt(1)}, @@ -251,13 +288,10 @@ func TestCoins(t *testing.T) { assert.True(t, good.IsAllGTE(empty), "Expected %v to be >= %v", good, empty) assert.False(t, good.IsAllLT(empty), "Expected %v to be < %v", good, empty) assert.True(t, empty.IsAllLT(good), "Expected %v to be < %v", empty, good) - assert.False(t, neg.IsPositive(), "Expected neg coins to not be positive: %v", neg) - assert.Zero(t, len(sum), "Expected 0 coins") assert.False(t, badSort1.IsValid(), "Coins are not sorted") assert.False(t, badSort2.IsValid(), "Coins are not sorted") assert.False(t, badAmt.IsValid(), "Coins cannot include 0 amounts") assert.False(t, dup.IsValid(), "Duplicate coin") - } func TestCoinsGT(t *testing.T) { @@ -314,32 +348,6 @@ func TestCoinsLTE(t *testing.T) { assert.True(t, Coins{}.IsAllLTE(Coins{{"A", one}})) } -func TestPlusCoins(t *testing.T) { - one := NewInt(1) - zero := NewInt(0) - negone := NewInt(-1) - two := NewInt(2) - - cases := []struct { - inputOne Coins - inputTwo Coins - expected Coins - }{ - {Coins{{"A", one}, {"B", one}}, Coins{{"A", one}, {"B", one}}, Coins{{"A", two}, {"B", two}}}, - {Coins{{"A", zero}, {"B", one}}, Coins{{"A", zero}, {"B", zero}}, Coins{{"B", one}}}, - {Coins{{"A", zero}, {"B", zero}}, Coins{{"A", zero}, {"B", zero}}, Coins(nil)}, - {Coins{{"A", one}, {"B", zero}}, Coins{{"A", negone}, {"B", zero}}, Coins(nil)}, - {Coins{{"A", negone}, {"B", zero}}, Coins{{"A", zero}, {"B", zero}}, Coins{{"A", negone}}}, - } - - for tcIndex, tc := range cases { - res := tc.inputOne.Plus(tc.inputTwo) - assert.True(t, res.IsValid()) - require.Equal(t, tc.expected, res, "sum of coins is incorrect, tc #%d", tcIndex) - } -} - -//Test the parsing of Coin and Coins func TestParse(t *testing.T) { one := NewInt(1) @@ -370,11 +378,9 @@ func TestParse(t *testing.T) { require.Equal(t, tc.expected, res, "coin parsing was incorrect, tc #%d", tcIndex) } } - } func TestSortCoins(t *testing.T) { - good := Coins{ NewInt64Coin("GAS", 1), NewInt64Coin("MINERAL", 1), @@ -424,7 +430,6 @@ func TestSortCoins(t *testing.T) { } func TestAmountOf(t *testing.T) { - case0 := Coins{} case1 := Coins{ NewInt64Coin("", 0), @@ -481,55 +486,3 @@ func TestAmountOf(t *testing.T) { assert.Equal(t, NewInt(tc.amountOfTREE), tc.coins.AmountOf("TREE")) } } - -func BenchmarkCoinsAdditionIntersect(b *testing.B) { - benchmarkingFunc := func(numCoinsA int, numCoinsB int) func(b *testing.B) { - return func(b *testing.B) { - coinsA := Coins(make([]Coin, numCoinsA)) - coinsB := Coins(make([]Coin, numCoinsB)) - for i := 0; i < numCoinsA; i++ { - coinsA[i] = NewCoin("COINZ_"+string(i), NewInt(int64(i))) - } - for i := 0; i < numCoinsB; i++ { - coinsB[i] = NewCoin("COINZ_"+string(i), NewInt(int64(i))) - } - b.ResetTimer() - for i := 0; i < b.N; i++ { - coinsA.Plus(coinsB) - } - } - } - - benchmarkSizes := [][]int{{1, 1}, {5, 5}, {5, 20}, {1, 1000}, {2, 1000}} - for i := 0; i < len(benchmarkSizes); i++ { - sizeA := benchmarkSizes[i][0] - sizeB := benchmarkSizes[i][1] - b.Run(fmt.Sprintf("sizes: A_%d, B_%d", sizeA, sizeB), benchmarkingFunc(sizeA, sizeB)) - } -} - -func BenchmarkCoinsAdditionNoIntersect(b *testing.B) { - benchmarkingFunc := func(numCoinsA int, numCoinsB int) func(b *testing.B) { - return func(b *testing.B) { - coinsA := Coins(make([]Coin, numCoinsA)) - coinsB := Coins(make([]Coin, numCoinsB)) - for i := 0; i < numCoinsA; i++ { - coinsA[i] = NewCoin("COINZ_"+string(numCoinsB+i), NewInt(int64(i))) - } - for i := 0; i < numCoinsB; i++ { - coinsB[i] = NewCoin("COINZ_"+string(i), NewInt(int64(i))) - } - b.ResetTimer() - for i := 0; i < b.N; i++ { - coinsA.Plus(coinsB) - } - } - } - - benchmarkSizes := [][]int{{1, 1}, {5, 5}, {5, 20}, {1, 1000}, {2, 1000}, {1000, 2}} - for i := 0; i < len(benchmarkSizes); i++ { - sizeA := benchmarkSizes[i][0] - sizeB := benchmarkSizes[i][1] - b.Run(fmt.Sprintf("sizes: A_%d, B_%d", sizeA, sizeB), benchmarkingFunc(sizeA, sizeB)) - } -} diff --git a/types/int.go b/types/int.go index 2083168e9..417a1763d 100644 --- a/types/int.go +++ b/types/int.go @@ -322,11 +322,11 @@ func NewUint(n uint64) Uint { // NewUintFromBigUint constructs Uint from big.Uint func NewUintFromBigInt(i *big.Int) Uint { - // Check overflow - if i.Sign() == -1 || i.Sign() == 1 && i.BitLen() > 256 { + res := Uint{i} + if UintOverflow(res) { panic("Uint overflow") } - return Uint{i} + return res } // NewUintFromString constructs Uint from string @@ -353,11 +353,12 @@ func NewUintWithDecimal(n uint64, dec int) Uint { i := new(big.Int) i.Mul(new(big.Int).SetUint64(n), exp) - // Check overflow - if i.Sign() == -1 || i.Sign() == 1 && i.BitLen() > 256 { + res := Uint{i} + if UintOverflow(res) { panic("NewUintWithDecimal() out of bound") } - return Uint{i} + + return res } // ZeroUint returns Uint value with zero @@ -408,8 +409,7 @@ func (i Uint) LT(i2 Uint) bool { // Add adds Uint from another func (i Uint) Add(i2 Uint) (res Uint) { res = Uint{add(i.i, i2.i)} - // Check overflow - if res.Sign() == -1 || res.Sign() == 1 && res.i.BitLen() > 256 { + if UintOverflow(res) { panic("Uint overflow") } return @@ -423,13 +423,23 @@ func (i Uint) AddRaw(i2 uint64) Uint { // Sub subtracts Uint from another func (i Uint) Sub(i2 Uint) (res Uint) { res = Uint{sub(i.i, i2.i)} - // Check overflow - if res.Sign() == -1 || res.Sign() == 1 && res.i.BitLen() > 256 { + if UintOverflow(res) { panic("Uint overflow") } return } +// SafeSub attempts to subtract one Uint from another. A boolean is also returned +// indicating if the result contains integer overflow. +func (i Uint) SafeSub(i2 Uint) (Uint, bool) { + res := Uint{sub(i.i, i2.i)} + if UintOverflow(res) { + return res, true + } + + return res, false +} + // SubRaw subtracts uint64 from Uint func (i Uint) SubRaw(i2 uint64) Uint { return i.Sub(NewUint(i2)) @@ -437,15 +447,15 @@ func (i Uint) SubRaw(i2 uint64) Uint { // Mul multiples two Uints func (i Uint) Mul(i2 Uint) (res Uint) { - // Check overflow if i.i.BitLen()+i2.i.BitLen()-1 > 256 { panic("Uint overflow") } + res = Uint{mul(i.i, i2.i)} - // Check overflow - if res.Sign() == -1 || res.Sign() == 1 && res.i.BitLen() > 256 { + if UintOverflow(res) { panic("Uint overflow") } + return } @@ -530,6 +540,12 @@ func (i *Uint) UnmarshalJSON(bz []byte) error { //__________________________________________________________________________ +// UintOverflow returns true if a given unsigned integer overflows and false +// otherwise. +func UintOverflow(x Uint) bool { + return x.i.Sign() == -1 || x.i.Sign() == 1 && x.i.BitLen() > 256 +} + // AddUint64Overflow performs the addition operation on two uint64 integers and // returns a boolean on whether or not the result overflows. func AddUint64Overflow(a, b uint64) (uint64, bool) { diff --git a/types/int_test.go b/types/int_test.go index 78bee66bf..9e189858c 100644 --- a/types/int_test.go +++ b/types/int_test.go @@ -591,6 +591,31 @@ func TestEncodingTableUint(t *testing.T) { } } +func TestSafeSub(t *testing.T) { + testCases := []struct { + x, y Uint + expected uint64 + overflow bool + }{ + {NewUint(0), NewUint(0), 0, false}, + {NewUint(10), NewUint(5), 5, false}, + {NewUint(5), NewUint(10), 5, true}, + {NewUint(math.MaxUint64), NewUint(0), math.MaxUint64, false}, + } + + for i, tc := range testCases { + res, overflow := tc.x.SafeSub(tc.y) + require.Equal( + t, tc.overflow, overflow, + "invalid overflow result; x: %s, y: %s, tc: #%d", tc.x, tc.y, i, + ) + require.Equal( + t, tc.expected, res.BigInt().Uint64(), + "invalid subtraction result; x: %s, y: %s, tc: #%d", tc.x, tc.y, i, + ) + } +} + func TestAddUint64Overflow(t *testing.T) { testCases := []struct { a, b uint64 diff --git a/x/auth/ante.go b/x/auth/ante.go index 7beb643c5..9a7a15e3e 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -281,24 +281,36 @@ func deductFees(acc Account, fee StdFee) (Account, sdk.Result) { coins := acc.GetCoins() feeAmount := fee.Amount - newCoins := coins.Minus(feeAmount) - if !newCoins.IsNotNegative() { + if !feeAmount.IsValid() { + return nil, sdk.ErrInsufficientFee(fmt.Sprintf("invalid fee amount: %s", feeAmount)).Result() + } + + newCoins, ok := coins.SafeMinus(feeAmount) + if ok { errMsg := fmt.Sprintf("%s < %s", coins, feeAmount) return nil, sdk.ErrInsufficientFunds(errMsg).Result() } + err := acc.SetCoins(newCoins) if err != nil { // Handle w/ #870 panic(err) } + return acc, sdk.Result{} } func ensureSufficientMempoolFees(ctx sdk.Context, stdTx StdTx) sdk.Result { // currently we use a very primitive gas pricing model with a constant gasPrice. // adjustFeesByGas handles calculating the amount of fees required based on the provided gas. - // TODO: Make the gasPrice not a constant, and account for tx size. - requiredFees := adjustFeesByGas(ctx.MinimumFees(), stdTx.Fee.Gas) + // + // TODO: + // - Make the gasPrice not a constant, and account for tx size. + // - Make Gas an unsigned integer and use tx basic validation + if stdTx.Fee.Gas <= 0 { + return sdk.ErrInternal(fmt.Sprintf("invalid gas supplied: %d", stdTx.Fee.Gas)).Result() + } + requiredFees := adjustFeesByGas(ctx.MinimumFees(), uint64(stdTx.Fee.Gas)) // NOTE: !A.IsAllGTE(B) is not the same as A.IsAllLT(B). if !ctx.MinimumFees().IsZero() && !stdTx.Fee.Amount.IsAllGTE(requiredFees) { diff --git a/x/auth/ante_test.go b/x/auth/ante_test.go index 0e8d13957..566892e07 100644 --- a/x/auth/ante_test.go +++ b/x/auth/ante_test.go @@ -708,7 +708,6 @@ func TestAdjustFeesByGas(t *testing.T) { }{ {"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)}}, - {"negative coins", args{sdk.Coins{sdk.NewInt64Coin("A", -10), sdk.NewInt64Coin("B", 10)}, 10000}, sdk.Coins{sdk.NewInt64Coin("B", 20)}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/x/bank/keeper.go b/x/bank/keeper.go index 3a33870e1..af930953f 100644 --- a/x/bank/keeper.go +++ b/x/bank/keeper.go @@ -182,11 +182,13 @@ func hasCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt s // 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 := oldCoins.Minus(amt) - if !newCoins.IsNotNegative() { + newCoins, hasNeg := oldCoins.SafeMinus(amt) + if hasNeg { return amt, nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", oldCoins, amt)) } + err := setCoins(ctx, am, addr, newCoins) tags := sdk.NewTags("sender", []byte(addr.String())) return newCoins, tags, err diff --git a/x/bank/msgs_test.go b/x/bank/msgs_test.go index 14e9a4c70..040d12bda 100644 --- a/x/bank/msgs_test.go +++ b/x/bank/msgs_test.go @@ -35,8 +35,6 @@ func TestInputValidation(t *testing.T) { emptyCoins := sdk.Coins{} emptyCoins2 := sdk.Coins{sdk.NewInt64Coin("eth", 0)} someEmptyCoins := sdk.Coins{sdk.NewInt64Coin("eth", 10), sdk.NewInt64Coin("atom", 0)} - minusCoins := sdk.Coins{sdk.NewInt64Coin("eth", -34)} - someMinusCoins := sdk.Coins{sdk.NewInt64Coin("atom", 20), sdk.NewInt64Coin("eth", -34)} unsortedCoins := sdk.Coins{sdk.NewInt64Coin("eth", 1), sdk.NewInt64Coin("atom", 1)} cases := []struct { @@ -52,8 +50,6 @@ func TestInputValidation(t *testing.T) { {false, NewInput(addr1, emptyCoins)}, // invalid coins {false, NewInput(addr1, emptyCoins2)}, // invalid coins {false, NewInput(addr1, someEmptyCoins)}, // invalid coins - {false, NewInput(addr1, minusCoins)}, // negative coins - {false, NewInput(addr1, someMinusCoins)}, // negative coins {false, NewInput(addr1, unsortedCoins)}, // unsorted coins } @@ -77,8 +73,6 @@ func TestOutputValidation(t *testing.T) { emptyCoins := sdk.Coins{} emptyCoins2 := sdk.Coins{sdk.NewInt64Coin("eth", 0)} someEmptyCoins := sdk.Coins{sdk.NewInt64Coin("eth", 10), sdk.NewInt64Coin("atom", 0)} - minusCoins := sdk.Coins{sdk.NewInt64Coin("eth", -34)} - someMinusCoins := sdk.Coins{sdk.NewInt64Coin("atom", 20), sdk.NewInt64Coin("eth", -34)} unsortedCoins := sdk.Coins{sdk.NewInt64Coin("eth", 1), sdk.NewInt64Coin("atom", 1)} cases := []struct { @@ -94,8 +88,6 @@ func TestOutputValidation(t *testing.T) { {false, NewOutput(addr1, emptyCoins)}, // invalid coins {false, NewOutput(addr1, emptyCoins2)}, // invalid coins {false, NewOutput(addr1, someEmptyCoins)}, // invalid coins - {false, NewOutput(addr1, minusCoins)}, // negative coins - {false, NewOutput(addr1, someMinusCoins)}, // negative coins {false, NewOutput(addr1, unsortedCoins)}, // unsorted coins } diff --git a/x/bank/simulation/msgs.go b/x/bank/simulation/msgs.go index f33c5e0c9..f1fd866c1 100644 --- a/x/bank/simulation/msgs.go +++ b/x/bank/simulation/msgs.go @@ -82,7 +82,7 @@ func createSingleInputSendMsg(r *rand.Rand, ctx sdk.Context, accs []simulation.A toAddr.String(), ) - coins := sdk.Coins{{initFromCoins[denomIndex].Denom, amt}} + coins := sdk.Coins{sdk.NewCoin(initFromCoins[denomIndex].Denom, amt)} msg = bank.MsgSend{ Inputs: []bank.Input{bank.NewInput(fromAcc.Address, coins)}, Outputs: []bank.Output{bank.NewOutput(toAddr, coins)}, diff --git a/x/gov/msgs_test.go b/x/gov/msgs_test.go index e488d2aba..4a661985c 100644 --- a/x/gov/msgs_test.go +++ b/x/gov/msgs_test.go @@ -13,7 +13,6 @@ import ( var ( coinsPos = sdk.Coins{sdk.NewInt64Coin(stakeTypes.DefaultBondDenom, 1000)} coinsZero = sdk.Coins{} - coinsNeg = sdk.Coins{sdk.NewInt64Coin(stakeTypes.DefaultBondDenom, -10000)} coinsPosNotAtoms = sdk.Coins{sdk.NewInt64Coin("foo", 10000)} coinsMulti = sdk.Coins{sdk.NewInt64Coin(stakeTypes.DefaultBondDenom, 1000), sdk.NewInt64Coin("foo", 10000)} ) @@ -40,7 +39,6 @@ func TestMsgSubmitProposal(t *testing.T) { {"Test Proposal", "the purpose of this proposal is to test", 0x05, addrs[0], coinsPos, false}, {"Test Proposal", "the purpose of this proposal is to test", ProposalTypeText, sdk.AccAddress{}, coinsPos, false}, {"Test Proposal", "the purpose of this proposal is to test", ProposalTypeText, addrs[0], coinsZero, true}, - {"Test Proposal", "the purpose of this proposal is to test", ProposalTypeText, addrs[0], coinsNeg, false}, {"Test Proposal", "the purpose of this proposal is to test", ProposalTypeText, addrs[0], coinsMulti, true}, } @@ -66,7 +64,6 @@ func TestMsgDeposit(t *testing.T) { {0, addrs[0], coinsPos, true}, {1, sdk.AccAddress{}, coinsPos, false}, {1, addrs[0], coinsZero, true}, - {1, addrs[0], coinsNeg, false}, {1, addrs[0], coinsMulti, true}, } diff --git a/x/slashing/handler_test.go b/x/slashing/handler_test.go index 9041bfe56..c77150535 100644 --- a/x/slashing/handler_test.go +++ b/x/slashing/handler_test.go @@ -20,7 +20,11 @@ func TestCannotUnjailUnlessJailed(t *testing.T) { got := stake.NewHandler(sk)(ctx, msg) require.True(t, got.IsOK()) stake.EndBlocker(ctx, sk) - require.Equal(t, ck.GetCoins(ctx, sdk.AccAddress(addr)), sdk.Coins{{sk.GetParams(ctx).BondDenom, initCoins.Sub(amt)}}) + + require.Equal( + t, ck.GetCoins(ctx, sdk.AccAddress(addr)), + sdk.Coins{sdk.NewCoin(sk.GetParams(ctx).BondDenom, initCoins.Sub(amt))}, + ) require.True(t, sdk.NewDecFromInt(amt).Equal(sk.Validator(ctx, addr).GetPower())) // assert non-jailed validator can't be unjailed diff --git a/x/slashing/keeper_test.go b/x/slashing/keeper_test.go index 94251ded3..e5318f292 100644 --- a/x/slashing/keeper_test.go +++ b/x/slashing/keeper_test.go @@ -35,7 +35,10 @@ func TestHandleDoubleSign(t *testing.T) { got := stake.NewHandler(sk)(ctx, NewTestMsgCreateValidator(operatorAddr, val, amt)) require.True(t, got.IsOK()) stake.EndBlocker(ctx, sk) - require.Equal(t, ck.GetCoins(ctx, sdk.AccAddress(operatorAddr)), sdk.Coins{{sk.GetParams(ctx).BondDenom, initCoins.Sub(amt)}}) + require.Equal( + t, ck.GetCoins(ctx, sdk.AccAddress(operatorAddr)), + sdk.Coins{sdk.NewCoin(sk.GetParams(ctx).BondDenom, initCoins.Sub(amt))}, + ) require.True(t, sdk.NewDecFromInt(amt).Equal(sk.Validator(ctx, operatorAddr).GetPower())) // handle a signature to set signing info @@ -76,7 +79,10 @@ func TestSlashingPeriodCap(t *testing.T) { require.True(t, got.IsOK()) stake.EndBlocker(ctx, sk) ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) - require.Equal(t, ck.GetCoins(ctx, sdk.AccAddress(operatorAddr)), sdk.Coins{{sk.GetParams(ctx).BondDenom, initCoins.Sub(amt)}}) + require.Equal( + t, ck.GetCoins(ctx, sdk.AccAddress(operatorAddr)), + sdk.Coins{sdk.NewCoin(sk.GetParams(ctx).BondDenom, initCoins.Sub(amt))}, + ) require.True(t, sdk.NewDecFromInt(amt).Equal(sk.Validator(ctx, operatorAddr).GetPower())) // handle a signature to set signing info @@ -140,8 +146,13 @@ func TestHandleAbsentValidator(t *testing.T) { got := sh(ctx, NewTestMsgCreateValidator(addr, val, amt)) require.True(t, got.IsOK()) stake.EndBlocker(ctx, sk) - require.Equal(t, ck.GetCoins(ctx, sdk.AccAddress(addr)), sdk.Coins{{sk.GetParams(ctx).BondDenom, initCoins.Sub(amt)}}) + + require.Equal( + t, ck.GetCoins(ctx, sdk.AccAddress(addr)), + sdk.Coins{sdk.NewCoin(sk.GetParams(ctx).BondDenom, initCoins.Sub(amt))}, + ) require.True(t, sdk.NewDecFromInt(amt).Equal(sk.Validator(ctx, addr).GetPower())) + // will exist since the validator has been bonded info, found := keeper.getValidatorSigningInfo(ctx, sdk.ConsAddress(val.Address())) require.True(t, found) @@ -296,7 +307,11 @@ func TestHandleNewValidator(t *testing.T) { got := sh(ctx, NewTestMsgCreateValidator(addr, val, sdk.NewInt(amt))) require.True(t, got.IsOK()) stake.EndBlocker(ctx, sk) - require.Equal(t, ck.GetCoins(ctx, sdk.AccAddress(addr)), sdk.Coins{{sk.GetParams(ctx).BondDenom, initCoins.SubRaw(amt)}}) + + require.Equal( + t, ck.GetCoins(ctx, sdk.AccAddress(addr)), + sdk.Coins{sdk.NewCoin(sk.GetParams(ctx).BondDenom, initCoins.SubRaw(amt))}, + ) require.Equal(t, sdk.NewDec(amt), sk.Validator(ctx, addr).GetPower()) // Now a validator, for two blocks diff --git a/x/slashing/tick_test.go b/x/slashing/tick_test.go index c6590c94e..932ac51a9 100644 --- a/x/slashing/tick_test.go +++ b/x/slashing/tick_test.go @@ -20,7 +20,10 @@ func TestBeginBlocker(t *testing.T) { got := stake.NewHandler(sk)(ctx, NewTestMsgCreateValidator(addr, pk, amt)) require.True(t, got.IsOK()) stake.EndBlocker(ctx, sk) - require.Equal(t, ck.GetCoins(ctx, sdk.AccAddress(addr)), sdk.Coins{{sk.GetParams(ctx).BondDenom, initCoins.Sub(amt)}}) + require.Equal( + t, ck.GetCoins(ctx, sdk.AccAddress(addr)), + sdk.Coins{sdk.NewCoin(sk.GetParams(ctx).BondDenom, initCoins.Sub(amt))}, + ) require.True(t, sdk.NewDecFromInt(amt).Equal(sk.Validator(ctx, addr).GetPower())) val := abci.Validator{ diff --git a/x/stake/types/msg_test.go b/x/stake/types/msg_test.go index 7e719f3ee..4b4055fca 100644 --- a/x/stake/types/msg_test.go +++ b/x/stake/types/msg_test.go @@ -12,7 +12,6 @@ import ( var ( coinPos = sdk.NewInt64Coin(DefaultBondDenom, 1000) coinZero = sdk.NewInt64Coin(DefaultBondDenom, 0) - coinNeg = sdk.NewInt64Coin(DefaultBondDenom, -10000) ) // test ValidateBasic for MsgCreateValidator @@ -34,8 +33,6 @@ func TestMsgCreateValidator(t *testing.T) { {"empty address", "a", "b", "c", "d", commission2, emptyAddr, pk1, coinPos, false}, {"empty pubkey", "a", "b", "c", "d", commission1, addr1, emptyPubkey, coinPos, true}, {"empty bond", "a", "b", "c", "d", commission2, addr1, pk1, coinZero, false}, - {"negative bond", "a", "b", "c", "d", commission2, addr1, pk1, coinNeg, false}, - {"negative bond", "a", "b", "c", "d", commission1, addr1, pk1, coinNeg, false}, } for _, tc := range tests { @@ -96,8 +93,6 @@ func TestMsgCreateValidatorOnBehalfOf(t *testing.T) { {"empty validator address", "a", "b", "c", "d", commission2, sdk.AccAddress(addr1), emptyAddr, pk2, coinPos, false}, {"empty pubkey", "a", "b", "c", "d", commission1, sdk.AccAddress(addr1), addr2, emptyPubkey, coinPos, true}, {"empty bond", "a", "b", "c", "d", commission2, sdk.AccAddress(addr1), addr2, pk2, coinZero, false}, - {"negative bond", "a", "b", "c", "d", commission1, sdk.AccAddress(addr1), addr2, pk2, coinNeg, false}, - {"negative bond", "a", "b", "c", "d", commission2, sdk.AccAddress(addr1), addr2, pk2, coinNeg, false}, } for _, tc := range tests { @@ -136,7 +131,6 @@ func TestMsgDelegate(t *testing.T) { {"empty delegator", sdk.AccAddress(emptyAddr), addr1, coinPos, false}, {"empty validator", sdk.AccAddress(addr1), emptyAddr, coinPos, false}, {"empty bond", sdk.AccAddress(addr1), addr2, coinZero, false}, - {"negative bond", sdk.AccAddress(addr1), addr2, coinNeg, false}, } for _, tc := range tests { From d8bbf85efaadd22dfb2dce7245ffa7db62f7daa0 Mon Sep 17 00:00:00 2001 From: Max Levy <35595512+maxim-levy@users.noreply.github.com> Date: Tue, 20 Nov 2018 19:03:11 +0900 Subject: [PATCH 9/9] Merge PR #2866: URL fixed Follow up on Documentation Structure Change and Cleanup (#2808) --- networks/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/networks/README.md b/networks/README.md index 221f6e385..9e948d4c2 100644 --- a/networks/README.md +++ b/networks/README.md @@ -3,4 +3,4 @@ Here contains the files required for automated deployment of either local or remote testnets. Doing so is best accomplished using the `make` targets. For more information, see the -[networks documentation](/docs/getting-started/networks.md) +[networks documentation](/docs/gaia/networks.md)