From 5624b1d7e54f9577a2cbcde97a8119cb6fb48ce6 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Wed, 15 Aug 2018 11:57:11 +0200 Subject: [PATCH 01/25] Update PRIORITIES.md --- docs/PRIORITIES.md | 84 ++++++++++++++++------------------------------ 1 file changed, 29 insertions(+), 55 deletions(-) diff --git a/docs/PRIORITIES.md b/docs/PRIORITIES.md index d628c136e..435ba02d3 100644 --- a/docs/PRIORITIES.md +++ b/docs/PRIORITIES.md @@ -1,78 +1,52 @@ +# High priority + ## Fees + - Collection - - Gas price based on parameter - - (which gets changed automatically) - - https://github.com/cosmos/cosmos-sdk/issues/1921 - - Per block gas usage as % - - Windowing function - - Block N, - - For Block N-x ~ N, get average of % - - Should take into account time. - - Standard for querying this price // needs to be used by UX. + - Simple flat fee set in-config by validators & full nodes - ref [#1921](https://github.com/cosmos/cosmos-sdk/issues/1921) - Distribution - - MVP: 1 week, 1 week for testing. + - "Piggy bank" fee distribution - ref [#1944](https://github.com/cosmos/cosmos-sdk/pull/1944) (spec) +- Reserve pool + - Collects in a special field for now, no spending ## Governance v2 -- V1 is just text proposals - - Software upgrade stuff - - https://github.com/cosmos/cosmos-sdk/issues/1734#issuecomment-407254938 - - https://github.com/cosmos/cosmos-sdk/issues/1079 -- We need to test auto-flipping w/ threshold voting power. -- Super simple: - - Only use text proposals - - On-chain mechanism for agreeing on when to "flip" to new functionality + +- Simple software upgrade proposals + - Implementation described in [#1079](https://github.com/cosmos/cosmos-sdk/issues/1079) + - Agree upon a block height to switch to new version ## Staking/Slashing/Stability + +- Miscellaneous minor staking issues + - [List here](https://github.com/cosmos/cosmos-sdk/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+label%3Astaking+label%3Aprelaunch) - Unbonding state for validators https://github.com/cosmos/cosmos-sdk/issues/1676 -- current: downtime, double signing during unbonding -- who gets slashed when -- needs review about edge cases -- need to communicate to everyone that lite has this edge case - - Issues: - - https://github.com/cosmos/cosmos-sdk/issues/1378 - - https://github.com/cosmos/cosmos-sdk/issues/1348 - - https://github.com/cosmos/cosmos-sdk/issues/1440 - * Est Difficulty: Hard - * _*Note:*_ This feature needs to be fully fleshed out. Will require a meeting between @jaekwon, @cwgoes, @rigel, @zaki, @bucky to discuss the issues. @jackzampolin to facilitate. +- Slashing period - ref [#2001](https://github.com/cosmos/cosmos-sdk/pull/2001) (spec) + - Various other slashing issues needing resolution - ref [#1256](https://github.com/cosmos/cosmos-sdk/issues/1256) +- Update staking/slashing for NextValSet change ## Vesting -- 24 accounts with NLocktime -- “No funds can be transferred before timelock” -- New atoms and such can be withdrawn right way -- Requires being able to send fees and inflation to new account + +- Single `VestingAccount` allowing delegation/voting but no withdrawals +- Ref [#1875](https://github.com/cosmos/cosmos-sdk/pull/1875) (spec) ## Multisig -- Make it work with Cli -- ADR -## Reserve Pool -- No withdrawing from it at launch +- Already implemented on TM side, need simple CLI interface -## Other: -- Need to update for NextValidatorSet - need to upgrade SDK for it -- Need to update for new ABCI changes - error string, tags are list of lists, proposer in header -- Inflation ? +## Other + +- Need to update for new ABCI changes - error string, tags are list of lists, proposer in header (Tendermint 0.24?) ## Gas -- Calculate gas -## Reward proposer -- Requires tendermint changes +- Benchmark gas, choose better constants # Lower priority -## Circuit Breaker -- Kinda needed for enabling txs. +## Governance -## Governance proposal changes -- V2 is parameter changes +- Circuit breaker (parameter change proposals, roughly the same implementation) -## Slashing/Stability -- tendermint evidence: we don’t yet slash byzantine signatures (signing at all) when not bonded. +## Documentation -# Other priority - -## gaiad // gaiacli -- Documentation // language - -## gaialite -- Documentation // language +- gaiad / gaiacli / gaialite documentation! From 53242681fe0edd44c3a6e28083f485139619432e Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Wed, 15 Aug 2018 12:50:07 +0200 Subject: [PATCH 02/25] Two bullet points --- docs/PRIORITIES.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/PRIORITIES.md b/docs/PRIORITIES.md index 435ba02d3..631d9b9ea 100644 --- a/docs/PRIORITIES.md +++ b/docs/PRIORITIES.md @@ -45,7 +45,8 @@ ## Governance -- Circuit breaker (parameter change proposals, roughly the same implementation) +- Circuit breaker +- Parameter change proposals (roughly the same implementation as circuit breaker) ## Documentation From f432c0c383af189360302c72d78c25bfa5e91784 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Wed, 22 Aug 2018 12:38:55 +0100 Subject: [PATCH 03/25] Simulate transactions before actual execution * Change --gas=0 semantic in order to enable gas auto estimate. * REST clients have been modified to simulate the execution of the tx first to then populate the context with the estimated gas amount returned by the simulation. * The simulation returns both an unadjusted gas estimate and an adjusted one. The adjustment is required to ensure that the ensuing execution doesn't fail due to state changes that might have occurred. Gas adjustment can be controlled via the CLI's --gas-adjustment flag. * Tiny refactorig of REST endpoints error handling. Closes: #1246 --- baseapp/baseapp.go | 35 +++++++---- client/context/context.go | 4 ++ client/context/query.go | 7 +-- client/flags.go | 6 +- client/lcd/lcd_test.go | 58 ++++++++++------- client/lcd/version.go | 2 +- client/utils/rest.go | 12 ++++ client/utils/utils.go | 61 +++++++++++++++++- client/utils/utils_test.go | 20 ++++++ cmd/gaia/cli_test/cli_test.go | 90 ++++++++++++++++---------- x/auth/ante.go | 6 +- x/auth/ante_test.go | 2 +- x/auth/client/context/context.go | 4 +- x/auth/client/rest/query.go | 13 ++-- x/bank/client/rest/sendtx.go | 33 +++++----- x/gov/client/rest/rest.go | 105 ++++++++++--------------------- x/gov/client/rest/util.go | 37 +++++------ x/ibc/client/rest/transfer.go | 31 ++++----- x/slashing/client/rest/tx.go | 33 +++++----- x/stake/client/rest/tx.go | 75 +++++++++------------- 20 files changed, 367 insertions(+), 267 deletions(-) create mode 100644 client/utils/rest.go create mode 100644 client/utils/utils_test.go diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 365d7b31f..2669aa9f3 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -495,13 +495,11 @@ func validateBasicTxMsgs(msgs []sdk.Msg) sdk.Error { func (app *BaseApp) getContextForAnte(mode runTxMode, txBytes []byte) (ctx sdk.Context) { // Get the context - if mode == runTxModeCheck || mode == runTxModeSimulate { - ctx = app.checkState.ctx.WithTxBytes(txBytes) - } else { - ctx = app.deliverState.ctx.WithTxBytes(txBytes) - ctx = ctx.WithSigningValidators(app.signedValidators) + ctx = getState(app, mode).ctx.WithTxBytes(txBytes) + if mode != runTxModeDeliver { + return } - + ctx = ctx.WithSigningValidators(app.signedValidators) return } @@ -567,6 +565,13 @@ func getState(app *BaseApp, mode runTxMode) *state { return app.deliverState } +func (app *BaseApp) applyTxMode(ctx sdk.Context, mode runTxMode) sdk.Context { + if mode != runTxModeSimulate { + return ctx + } + return ctx.WithMultiStore(getState(app, runTxModeSimulate).CacheMultiStore()) +} + // 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. @@ -575,7 +580,9 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk // 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.applyTxMode(ctx, mode) defer func() { if r := recover(); r != nil { @@ -594,9 +601,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk }() var msgs = tx.GetMsgs() - - err := validateBasicTxMsgs(msgs) - if err != nil { + if err := validateBasicTxMsgs(msgs); err != nil { return err.Result() } @@ -613,9 +618,15 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk gasWanted = result.GasWanted } + if mode == runTxModeSimulate { + result = app.runMsgs(ctx, msgs, mode) + result.GasWanted = gasWanted + return + } + // Keep the state in a transient CacheWrap in case processing the messages // fails. - msCache := getState(app, mode).CacheMultiStore() + msCache = getState(app, mode).CacheMultiStore() if msCache.TracingEnabled() { msCache = msCache.WithTracingContext(sdk.TraceContext( map[string]interface{}{"txHash": cmn.HexBytes(tmhash.Sum(txBytes)).String()}, @@ -626,8 +637,8 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk result = app.runMsgs(ctx, msgs, mode) result.GasWanted = gasWanted - // only update state if all messages pass and we're not in a simulation - if result.IsOK() && mode != runTxModeSimulate { + // only update state if all messages pass + if result.IsOK() { msCache.Write() } diff --git a/client/context/context.go b/client/context/context.go index 1b0443b0c..743c92355 100644 --- a/client/context/context.go +++ b/client/context/context.go @@ -22,6 +22,8 @@ type CLIContext struct { Client rpcclient.Client Logger io.Writer Height int64 + Gas int64 + GasAdjustment float64 NodeURI string FromAddressName string AccountStore string @@ -48,6 +50,8 @@ func NewCLIContext() CLIContext { AccountStore: ctxAccStoreName, FromAddressName: viper.GetString(client.FlagFrom), Height: viper.GetInt64(client.FlagHeight), + Gas: viper.GetInt64(client.FlagGas), + GasAdjustment: viper.GetFloat64(client.FlagGasAdjustment), TrustNode: viper.GetBool(client.FlagTrustNode), UseLedger: viper.GetBool(client.FlagUseLedger), Async: viper.GetBool(client.FlagAsync), diff --git a/client/context/query.go b/client/context/query.go index 68676f741..e526c0abb 100644 --- a/client/context/query.go +++ b/client/context/query.go @@ -10,7 +10,6 @@ import ( "github.com/pkg/errors" - "github.com/tendermint/tendermint/libs/common" cmn "github.com/tendermint/tendermint/libs/common" rpcclient "github.com/tendermint/tendermint/rpc/client" ctypes "github.com/tendermint/tendermint/rpc/core/types" @@ -27,8 +26,8 @@ func (ctx CLIContext) GetNode() (rpcclient.Client, error) { } // Query performs a query for information about the connected node. -func (ctx CLIContext) Query(path string) (res []byte, err error) { - return ctx.query(path, nil) +func (ctx CLIContext) Query(path string, data cmn.HexBytes) (res []byte, err error) { + return ctx.query(path, data) } // Query information about the connected node with a data payload @@ -284,7 +283,7 @@ func (ctx CLIContext) ensureBroadcastTx(txBytes []byte) error { // query performs a query from a Tendermint node with the provided store name // and path. -func (ctx CLIContext) query(path string, key common.HexBytes) (res []byte, err error) { +func (ctx CLIContext) query(path string, key cmn.HexBytes) (res []byte, err error) { node, err := ctx.GetNode() if err != nil { return res, err diff --git a/client/flags.go b/client/flags.go index b02078905..d128d32f6 100644 --- a/client/flags.go +++ b/client/flags.go @@ -4,11 +4,14 @@ import "github.com/spf13/cobra" // nolint const ( + DefaultGasAdjustment = 0 + FlagUseLedger = "ledger" FlagChainID = "chain-id" FlagNode = "node" FlagHeight = "height" FlagGas = "gas" + FlagGasAdjustment = "gas-adjustment" FlagTrustNode = "trust-node" FlagFrom = "from" FlagName = "name" @@ -49,7 +52,8 @@ func PostCommands(cmds ...*cobra.Command) []*cobra.Command { c.Flags().String(FlagChainID, "", "Chain ID of tendermint node") c.Flags().String(FlagNode, "tcp://localhost:26657", ": to tendermint rpc interface for this chain") c.Flags().Bool(FlagUseLedger, false, "Use a connected Ledger device") - c.Flags().Int64(FlagGas, 200000, "gas limit to set per-transaction") + c.Flags().Int64(FlagGas, 0, "gas limit to set per-transaction; set to 0 to calculate required gas automatically") + c.Flags().Float64(FlagGasAdjustment, DefaultGasAdjustment, "gas adjustment to be applied on the estimate returned by the tx simulation") c.Flags().Bool(FlagAsync, false, "broadcast transactions asynchronously") c.Flags().Bool(FlagJson, false, "return output in json format") c.Flags().Bool(FlagPrintResponse, true, "return tx response (only works with async = false)") diff --git a/client/lcd/lcd_test.go b/client/lcd/lcd_test.go index 2e7ab0af7..2db2f6d61 100644 --- a/client/lcd/lcd_test.go +++ b/client/lcd/lcd_test.go @@ -263,6 +263,10 @@ func TestCoinSend(t *testing.T) { require.Equal(t, "steak", mycoins.Denom) require.Equal(t, int64(1), mycoins.Amount.Int64()) + + // test failure with too little gas + res, body, _ = doSendWithGas(t, port, seed, name, password, addr, 100) + require.Equal(t, http.StatusInternalServerError, res.StatusCode, body) } func TestIBCTransfer(t *testing.T) { @@ -712,7 +716,7 @@ func getAccount(t *testing.T, port string, addr sdk.AccAddress) auth.Account { return acc } -func doSend(t *testing.T, port, seed, name, password string, addr sdk.AccAddress) (receiveAddr sdk.AccAddress, resultTx ctypes.ResultBroadcastTxCommit) { +func doSendWithGas(t *testing.T, port, seed, name, password string, addr sdk.AccAddress, gas int64) (res *http.Response, body string, receiveAddr sdk.AccAddress) { // create receive address kb := client.MockKeyBase() @@ -730,19 +734,36 @@ func doSend(t *testing.T, port, seed, name, password string, addr sdk.AccAddress panic(err) } - jsonStr := []byte(fmt.Sprintf(`{ - "name":"%s", - "password":"%s", - "account_number":"%d", - "sequence":"%d", - "gas": "10000", - "amount":[%s], - "chain_id":"%s" - }`, name, password, accnum, sequence, coinbz, chainID)) - res, body := Request(t, port, "POST", fmt.Sprintf("/accounts/%s/send", receiveAddr), jsonStr) + jsonStr := func() []byte { + if gas > 0 { + return []byte(fmt.Sprintf(`{ + "name":"%s", + "password":"%s", + "account_number":"%d", + "sequence":"%d", + "amount":[%s], + "chain_id":"%s", + "gas":"%v" + }`, name, password, accnum, sequence, coinbz, chainID, gas)) + } + return []byte(fmt.Sprintf(`{ + "name":"%s", + "password":"%s", + "account_number":"%d", + "sequence":"%d", + "amount":[%s], + "chain_id":"%s" + }`, name, password, accnum, sequence, coinbz, chainID)) + }() + res, body = Request(t, port, "POST", fmt.Sprintf("/accounts/%s/send", receiveAddr), jsonStr) + return +} + +func doSend(t *testing.T, port, seed, name, password string, addr sdk.AccAddress) (receiveAddr sdk.AccAddress, resultTx ctypes.ResultBroadcastTxCommit) { + res, body, receiveAddr := doSendWithGas(t, port, seed, name, password, addr, 0) require.Equal(t, http.StatusOK, res.StatusCode, body) - err = cdc.UnmarshalJSON([]byte(body), &resultTx) + err := cdc.UnmarshalJSON([]byte(body), &resultTx) require.Nil(t, err) return receiveAddr, resultTx @@ -768,7 +789,6 @@ func doIBCTransfer(t *testing.T, port, seed, name, password string, addr sdk.Acc "password": "%s", "account_number":"%d", "sequence": "%d", - "gas": "100000", "src_chain_id": "%s", "amount":[ { @@ -887,7 +907,6 @@ func doDelegate(t *testing.T, port, seed, name, password string, delegatorAddr, "password": "%s", "account_number": "%d", "sequence": "%d", - "gas": "10000", "chain_id": "%s", "delegations": [ { @@ -925,7 +944,6 @@ func doBeginUnbonding(t *testing.T, port, seed, name, password string, "password": "%s", "account_number": "%d", "sequence": "%d", - "gas": "20000", "chain_id": "%s", "delegations": [], "begin_unbondings": [ @@ -964,7 +982,6 @@ func doBeginRedelegation(t *testing.T, port, seed, name, password string, "password": "%s", "account_number": "%d", "sequence": "%d", - "gas": "10000", "chain_id": "%s", "delegations": [], "begin_unbondings": [], @@ -1116,8 +1133,7 @@ func doSubmitProposal(t *testing.T, port, seed, name, password string, proposerA "password": "%s", "chain_id": "%s", "account_number":"%d", - "sequence":"%d", - "gas":"100000" + "sequence":"%d" } }`, proposerAddr, name, password, chainID, accnum, sequence)) res, body := Request(t, port, "POST", "/gov/proposals", jsonStr) @@ -1147,8 +1163,7 @@ func doDeposit(t *testing.T, port, seed, name, password string, proposerAddr sdk "password": "%s", "chain_id": "%s", "account_number":"%d", - "sequence": "%d", - "gas":"100000" + "sequence": "%d" } }`, proposerAddr, name, password, chainID, accnum, sequence)) res, body := Request(t, port, "POST", fmt.Sprintf("/gov/proposals/%d/deposits", proposalID), jsonStr) @@ -1178,8 +1193,7 @@ func doVote(t *testing.T, port, seed, name, password string, proposerAddr sdk.Ac "password": "%s", "chain_id": "%s", "account_number": "%d", - "sequence": "%d", - "gas":"100000" + "sequence": "%d" } }`, proposerAddr, name, password, chainID, accnum, sequence)) res, body := Request(t, port, "POST", fmt.Sprintf("/gov/proposals/%d/votes", proposalID), jsonStr) diff --git a/client/lcd/version.go b/client/lcd/version.go index 377d7ca26..a124388e6 100644 --- a/client/lcd/version.go +++ b/client/lcd/version.go @@ -17,7 +17,7 @@ func CLIVersionRequestHandler(w http.ResponseWriter, r *http.Request) { // connected node version REST handler endpoint func NodeVersionRequestHandler(cliCtx context.CLIContext) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - version, err := cliCtx.Query("/app/version") + version, err := cliCtx.Query("/app/version", nil) if err != nil { w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(fmt.Sprintf("Could't query version. Error: %s", err.Error()))) diff --git a/client/utils/rest.go b/client/utils/rest.go new file mode 100644 index 000000000..0e2640312 --- /dev/null +++ b/client/utils/rest.go @@ -0,0 +1,12 @@ +package utils + +import ( + "net/http" +) + +// WriteErrorResponse prepares and writes a HTTP error +// given a status code and an error message. +func WriteErrorResponse(w *http.ResponseWriter, status int, msg string) { + (*w).WriteHeader(status) + (*w).Write([]byte(msg)) +} diff --git a/client/utils/utils.go b/client/utils/utils.go index 8a058b56f..9f06dda1d 100644 --- a/client/utils/utils.go +++ b/client/utils/utils.go @@ -1,12 +1,21 @@ package utils import ( + "fmt" + "os" + "github.com/cosmos/cosmos-sdk/client/context" "github.com/cosmos/cosmos-sdk/client/keys" sdk "github.com/cosmos/cosmos-sdk/types" authctx "github.com/cosmos/cosmos-sdk/x/auth/client/context" + amino "github.com/tendermint/go-amino" ) +// DefaultGasAdjustment is applied to gas estimates to avoid tx +// execution failures due to state changes that might +// occur between the tx simulation and the actual run. +const DefaultGasAdjustment = 1.2 + // SendTx implements a auxiliary handler that facilitates sending a series of // messages in a signed transaction given a TxContext and a QueryContext. It // ensures that the account exists, has a proper number and sequence set. In @@ -49,12 +58,62 @@ func SendTx(txCtx authctx.TxContext, cliCtx context.CLIContext, msgs []sdk.Msg) return err } + txCtx, err = enrichCtxWithGasIfGasAuto(txCtx, cliCtx, cliCtx.FromAddressName, passphrase, msgs) + if err != nil { + return err + } + // build and sign the transaction txBytes, err := txCtx.BuildAndSign(cliCtx.FromAddressName, passphrase, msgs) if err != nil { return err } - // broadcast to a Tendermint node return cliCtx.EnsureBroadcastTx(txBytes) } + +func enrichCtxWithGasIfGasAuto(txCtx authctx.TxContext, cliCtx context.CLIContext, name, passphrase string, msgs []sdk.Msg) (authctx.TxContext, error) { + if cliCtx.Gas == 0 { + return EnrichTxContextWithGas(txCtx, cliCtx, name, passphrase, msgs) + } + return txCtx, nil +} + +// EnrichTxContextWithGas simulates the execution of a transaction to +// then populate the relevant TxContext.Gas field with the estimate +// obtained by the query. +func EnrichTxContextWithGas(txCtx authctx.TxContext, cliCtx context.CLIContext, name, passphrase string, msgs []sdk.Msg) (authctx.TxContext, error) { + txCtxSimulation := txCtx.WithGas(0) + txBytes, err := txCtxSimulation.BuildAndSign(name, passphrase, msgs) + if err != nil { + return txCtx, err + } + // run a simulation (via /app/simulate query) to + // estimate gas and update TxContext accordingly + rawRes, err := cliCtx.Query("/app/simulate", txBytes) + if err != nil { + return txCtx, err + } + estimate, err := parseQueryResponse(cliCtx.Codec, rawRes) + if err != nil { + return txCtx, err + } + adjusted := adjustGasEstimate(estimate, cliCtx.GasAdjustment) + fmt.Fprintf(os.Stderr, "gas: [estimated = %v] [adjusted = %v]\n", estimate, adjusted) + return txCtx.WithGas(adjusted), nil +} + +func adjustGasEstimate(estimate int64, adjustment float64) int64 { + if adjustment == 0 { + return int64(DefaultGasAdjustment * float64(estimate)) + } + return int64(adjustment * float64(estimate)) +} + +func parseQueryResponse(cdc *amino.Codec, rawRes []byte) (int64, error) { + var simulationResult sdk.Result + if err := cdc.UnmarshalBinary(rawRes, &simulationResult); err != nil { + return 0, err + } + return simulationResult.GasUsed, nil +} diff --git a/client/utils/utils_test.go b/client/utils/utils_test.go new file mode 100644 index 000000000..d9ea44488 --- /dev/null +++ b/client/utils/utils_test.go @@ -0,0 +1,20 @@ +package utils + +import ( + "testing" + + "github.com/cosmos/cosmos-sdk/cmd/gaia/app" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/assert" +) + +func TestParseQueryResponse(t *testing.T) { + cdc := app.MakeCodec() + sdkResBytes := cdc.MustMarshalBinary(sdk.Result{GasUsed: 10}) + gas, err := parseQueryResponse(cdc, sdkResBytes) + assert.Equal(t, gas, int64(10)) + assert.Nil(t, err) + gas, err = parseQueryResponse(cdc, []byte("fuzzy")) + assert.Equal(t, gas, int64(0)) + assert.NotNil(t, err) +} diff --git a/cmd/gaia/cli_test/cli_test.go b/cmd/gaia/cli_test/cli_test.go index 47a1949dd..f794e5140 100644 --- a/cmd/gaia/cli_test/cli_test.go +++ b/cmd/gaia/cli_test/cli_test.go @@ -34,16 +34,7 @@ func init() { } func TestGaiaCLISend(t *testing.T) { - tests.ExecuteT(t, fmt.Sprintf("gaiad --home=%s unsafe-reset-all", gaiadHome), "") - executeWrite(t, fmt.Sprintf("gaiacli keys delete --home=%s foo", gaiacliHome), app.DefaultKeyPass) - executeWrite(t, fmt.Sprintf("gaiacli keys delete --home=%s bar", gaiacliHome), app.DefaultKeyPass) - - chainID := executeInit(t, fmt.Sprintf("gaiad init -o --name=foo --home=%s --home-client=%s", gaiadHome, gaiacliHome)) - executeWrite(t, fmt.Sprintf("gaiacli keys add --home=%s bar", gaiacliHome), app.DefaultKeyPass) - - // get a free port, also setup some common flags - servAddr, port, err := server.FreeTCPAddr() - require.NoError(t, err) + chainID, servAddr, port := initializeFixtures(t) flags := fmt.Sprintf("--home=%s --node=%v --chain-id=%v", gaiacliHome, servAddr, chainID) // start gaiad server @@ -86,16 +77,42 @@ func TestGaiaCLISend(t *testing.T) { require.Equal(t, int64(20), fooAcc.GetCoins().AmountOf("steak").Int64()) } -func TestGaiaCLICreateValidator(t *testing.T) { - tests.ExecuteT(t, fmt.Sprintf("gaiad --home=%s unsafe-reset-all", gaiadHome), "") - executeWrite(t, fmt.Sprintf("gaiacli keys delete --home=%s foo", gaiacliHome), app.DefaultKeyPass) - executeWrite(t, fmt.Sprintf("gaiacli keys delete --home=%s bar", gaiacliHome), app.DefaultKeyPass) - chainID := executeInit(t, fmt.Sprintf("gaiad init -o --name=foo --home=%s --home-client=%s", gaiadHome, gaiacliHome)) - executeWrite(t, fmt.Sprintf("gaiacli keys add --home=%s bar", gaiacliHome), app.DefaultKeyPass) +func TestGaiaCLIGasAuto(t *testing.T) { + chainID, servAddr, port := initializeFixtures(t) + flags := fmt.Sprintf("--home=%s --node=%v --chain-id=%v", gaiacliHome, servAddr, chainID) - // get a free port, also setup some common flags - servAddr, port, err := server.FreeTCPAddr() - require.NoError(t, err) + // start gaiad server + proc := tests.GoExecuteTWithStdout(t, fmt.Sprintf("gaiad start --home=%s --rpc.laddr=%v", gaiadHome, servAddr)) + + defer proc.Stop(false) + tests.WaitForTMStart(port) + tests.WaitForNextNBlocksTM(2, port) + + fooAddr, _ := executeGetAddrPK(t, fmt.Sprintf("gaiacli keys show foo --output=json --home=%s", gaiacliHome)) + barAddr, _ := executeGetAddrPK(t, fmt.Sprintf("gaiacli keys show bar --output=json --home=%s", gaiacliHome)) + + fooAcc := executeGetAccount(t, fmt.Sprintf("gaiacli account %s %v", fooAddr, flags)) + require.Equal(t, int64(50), fooAcc.GetCoins().AmountOf("steak").Int64()) + + // Test failure with auto gas disabled and very little gas set by hand + success := executeWrite(t, fmt.Sprintf("gaiacli send %v --gas=10 --amount=10steak --to=%s --from=foo", flags, barAddr), app.DefaultKeyPass) + require.False(t, success) + tests.WaitForNextNBlocksTM(2, port) + // Check state didn't change + fooAcc = executeGetAccount(t, fmt.Sprintf("gaiacli account %s %v", fooAddr, flags)) + require.Equal(t, int64(50), fooAcc.GetCoins().AmountOf("steak").Int64()) + + // Enable auto gas + success = executeWrite(t, fmt.Sprintf("gaiacli send %v --gas=0 --amount=10steak --to=%s --from=foo", flags, barAddr), app.DefaultKeyPass) + require.True(t, success) + tests.WaitForNextNBlocksTM(2, port) + // Check state has changed accordingly + fooAcc = executeGetAccount(t, fmt.Sprintf("gaiacli account %s %v", fooAddr, flags)) + require.Equal(t, int64(40), fooAcc.GetCoins().AmountOf("steak").Int64()) +} + +func TestGaiaCLICreateValidator(t *testing.T) { + chainID, servAddr, port := initializeFixtures(t) flags := fmt.Sprintf("--home=%s --node=%v --chain-id=%v", gaiacliHome, servAddr, chainID) // start gaiad server @@ -168,15 +185,7 @@ func TestGaiaCLICreateValidator(t *testing.T) { } func TestGaiaCLISubmitProposal(t *testing.T) { - tests.ExecuteT(t, fmt.Sprintf("gaiad --home=%s unsafe-reset-all", gaiadHome), "") - executeWrite(t, fmt.Sprintf("gaiacli keys delete --home=%s foo", gaiacliHome), app.DefaultKeyPass) - executeWrite(t, fmt.Sprintf("gaiacli keys delete --home=%s bar", gaiacliHome), app.DefaultKeyPass) - chainID := executeInit(t, fmt.Sprintf("gaiad init -o --name=foo --home=%s --home-client=%s", gaiadHome, gaiacliHome)) - executeWrite(t, fmt.Sprintf("gaiacli keys add --home=%s bar", gaiacliHome), app.DefaultKeyPass) - - // get a free port, also setup some common flags - servAddr, port, err := server.FreeTCPAddr() - require.NoError(t, err) + chainID, servAddr, port := initializeFixtures(t) flags := fmt.Sprintf("--home=%s --node=%v --chain-id=%v", gaiacliHome, servAddr, chainID) // start gaiad server @@ -277,10 +286,29 @@ func getTestingHomeDirs() (string, string) { return gaiadHome, gaiacliHome } +func initializeFixtures(t *testing.T) (chainID, servAddr, port string) { + tests.ExecuteT(t, fmt.Sprintf("gaiad --home=%s unsafe-reset-all", gaiadHome), "") + executeWrite(t, fmt.Sprintf("gaiacli keys delete --home=%s foo", gaiacliHome), app.DefaultKeyPass) + executeWrite(t, fmt.Sprintf("gaiacli keys delete --home=%s bar", gaiacliHome), app.DefaultKeyPass) + + chainID = executeInit(t, fmt.Sprintf("gaiad init -o --name=foo --home=%s --home-client=%s", gaiadHome, gaiacliHome)) + executeWrite(t, fmt.Sprintf("gaiacli keys add --home=%s bar", gaiacliHome), app.DefaultKeyPass) + + // get a free port, also setup some common flags + servAddr, port, err := server.FreeTCPAddr() + require.NoError(t, err) + return +} + //___________________________________________________________________________________ // executors -func executeWrite(t *testing.T, cmdStr string, writes ...string) bool { +func executeWrite(t *testing.T, cmdStr string, writes ...string) (exitSuccess bool) { + exitSuccess, _, _ = executeWriteRetStdStreams(t, cmdStr, writes...) + return +} + +func executeWriteRetStdStreams(t *testing.T, cmdStr string, writes ...string) (bool, string, string) { proc := tests.GoExecuteT(t, cmdStr) for _, write := range writes { @@ -300,9 +328,7 @@ func executeWrite(t *testing.T, cmdStr string, writes ...string) bool { } proc.Wait() - return proc.ExitState.Success() - // bz := proc.StdoutBuffer.Bytes() - // fmt.Println("EXEC WRITE", string(bz)) + return proc.ExitState.Success(), string(stdout), string(stderr) } func executeInit(t *testing.T, cmdStr string) (chainID string) { diff --git a/x/auth/ante.go b/x/auth/ante.go index 078f376e2..dd291a4a6 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -35,7 +35,11 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { } // set the gas meter - newCtx = ctx.WithGasMeter(sdk.NewGasMeter(stdTx.Fee.Gas)) + if stdTx.Fee.Gas == 0 { + newCtx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) + } else { + newCtx = ctx.WithGasMeter(sdk.NewGasMeter(stdTx.Fee.Gas)) + } // AnteHandlers must have their own defer/recover in order // for the BaseApp to know how much gas was used! diff --git a/x/auth/ante_test.go b/x/auth/ante_test.go index 5fa04d848..25d31b067 100644 --- a/x/auth/ante_test.go +++ b/x/auth/ante_test.go @@ -360,7 +360,7 @@ func TestAnteHandlerMemoGas(t *testing.T) { var tx sdk.Tx msg := newTestMsg(addr1) privs, accnums, seqs := []crypto.PrivKey{priv1}, []int64{0}, []int64{0} - fee := NewStdFee(0, sdk.NewInt64Coin("atom", 0)) + fee := NewStdFee(1, sdk.NewInt64Coin("atom", 0)) // tx does not have enough gas tx = newTestTx(ctx, []sdk.Msg{msg}, privs, accnums, seqs, fee) diff --git a/x/auth/client/context/context.go b/x/auth/client/context/context.go index 1cfa435ee..8d0a94136 100644 --- a/x/auth/client/context/context.go +++ b/x/auth/client/context/context.go @@ -110,9 +110,7 @@ func (ctx TxContext) Build(msgs []sdk.Msg) (auth.StdSignMsg, error) { Sequence: ctx.Sequence, Memo: ctx.Memo, Msgs: msgs, - - // TODO: run simulate to estimate gas? - Fee: auth.NewStdFee(ctx.Gas, fee), + Fee: auth.NewStdFee(ctx.Gas, fee), }, nil } diff --git a/x/auth/client/rest/query.go b/x/auth/client/rest/query.go index 431d3d27d..07b109d40 100644 --- a/x/auth/client/rest/query.go +++ b/x/auth/client/rest/query.go @@ -5,6 +5,7 @@ import ( "net/http" "github.com/cosmos/cosmos-sdk/client/context" + "github.com/cosmos/cosmos-sdk/client/utils" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/wire" "github.com/cosmos/cosmos-sdk/x/auth" @@ -32,15 +33,13 @@ func QueryAccountRequestHandlerFn( addr, err := sdk.AccAddressFromBech32(bech32addr) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } res, err := cliCtx.QueryStore(auth.AddressStoreKey(addr), storeName) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("couldn't query account. Error: %s", err.Error()))) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, fmt.Sprintf("couldn't query account. Error: %s", err.Error())) return } @@ -53,16 +52,14 @@ func QueryAccountRequestHandlerFn( // decode the value account, err := decoder(res) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("couldn't parse query result. Result: %s. Error: %s", res, err.Error()))) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, fmt.Sprintf("couldn't parse query result. Result: %s. Error: %s", res, err.Error())) return } // print out whole account output, err := cdc.MarshalJSON(account) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("couldn't marshall query result. Error: %s", err.Error()))) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, fmt.Sprintf("couldn't marshall query result. Error: %s", err.Error())) return } diff --git a/x/bank/client/rest/sendtx.go b/x/bank/client/rest/sendtx.go index e060a3bb4..d27be38b5 100644 --- a/x/bank/client/rest/sendtx.go +++ b/x/bank/client/rest/sendtx.go @@ -5,6 +5,7 @@ import ( "net/http" "github.com/cosmos/cosmos-sdk/client/context" + "github.com/cosmos/cosmos-sdk/client/utils" "github.com/cosmos/cosmos-sdk/crypto/keys" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/wire" @@ -47,37 +48,32 @@ func SendRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx context.CLICo to, err := sdk.AccAddressFromBech32(bech32addr) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } var m sendBody body, err := ioutil.ReadAll(r.Body) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } err = msgCdc.UnmarshalJSON(body, &m) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } info, err := kb.Get(m.LocalAccountName) if err != nil { - w.WriteHeader(http.StatusUnauthorized) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusUnauthorized, err.Error()) return } // build message msg := client.BuildMsg(sdk.AccAddress(info.GetPubKey().Address()), to, m.Amount) if err != nil { // XXX rechecking same error ? - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } @@ -89,24 +85,29 @@ func SendRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx context.CLICo Sequence: m.Sequence, } + if m.Gas == 0 { + txCtx, err = utils.EnrichTxContextWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, []sdk.Msg{msg}) + if err != nil { + utils.WriteErrorResponse(&w, http.StatusUnauthorized, err.Error()) + return + } + } + txBytes, err := txCtx.BuildAndSign(m.LocalAccountName, m.Password, []sdk.Msg{msg}) if err != nil { - w.WriteHeader(http.StatusUnauthorized) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusUnauthorized, err.Error()) return } res, err := cliCtx.BroadcastTx(txBytes) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } output, err := wire.MarshalJSONIndent(cdc, res) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } diff --git a/x/gov/client/rest/rest.go b/x/gov/client/rest/rest.go index 5cdc7bda2..d6a9199d4 100644 --- a/x/gov/client/rest/rest.go +++ b/x/gov/client/rest/rest.go @@ -5,6 +5,7 @@ import ( "net/http" "github.com/cosmos/cosmos-sdk/client/context" + "github.com/cosmos/cosmos-sdk/client/utils" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/wire" "github.com/cosmos/cosmos-sdk/x/gov" @@ -76,7 +77,7 @@ func postProposalHandlerFn(cdc *wire.Codec, cliCtx context.CLIContext) http.Hand msg := gov.NewMsgSubmitProposal(req.Title, req.Description, req.ProposalType, req.Proposer, req.InitialDeposit) err = msg.ValidateBasic() if err != nil { - writeErr(&w, http.StatusBadRequest, err.Error()) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } @@ -90,10 +91,8 @@ func depositHandlerFn(cdc *wire.Codec, cliCtx context.CLIContext) http.HandlerFu strProposalID := vars[RestProposalID] if len(strProposalID) == 0 { - w.WriteHeader(http.StatusBadRequest) err := errors.New("proposalId required but not specified") - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } @@ -115,7 +114,7 @@ func depositHandlerFn(cdc *wire.Codec, cliCtx context.CLIContext) http.HandlerFu msg := gov.NewMsgDeposit(req.Depositer, proposalID, req.Amount) err = msg.ValidateBasic() if err != nil { - writeErr(&w, http.StatusBadRequest, err.Error()) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } @@ -129,10 +128,8 @@ func voteHandlerFn(cdc *wire.Codec, cliCtx context.CLIContext) http.HandlerFunc strProposalID := vars[RestProposalID] if len(strProposalID) == 0 { - w.WriteHeader(http.StatusBadRequest) err := errors.New("proposalId required but not specified") - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } @@ -154,7 +151,7 @@ func voteHandlerFn(cdc *wire.Codec, cliCtx context.CLIContext) http.HandlerFunc msg := gov.NewMsgVote(req.Voter, proposalID, req.Option) err = msg.ValidateBasic() if err != nil { - writeErr(&w, http.StatusBadRequest, err.Error()) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } @@ -168,10 +165,8 @@ func queryProposalHandlerFn(cdc *wire.Codec) http.HandlerFunc { strProposalID := vars[RestProposalID] if len(strProposalID) == 0 { - w.WriteHeader(http.StatusBadRequest) err := errors.New("proposalId required but not specified") - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } @@ -188,16 +183,13 @@ func queryProposalHandlerFn(cdc *wire.Codec) http.HandlerFunc { bz, err := cdc.MarshalJSON(params) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } res, err := cliCtx.QueryWithData("custom/gov/proposal", bz) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } @@ -212,10 +204,8 @@ func queryDepositHandlerFn(cdc *wire.Codec) http.HandlerFunc { bechDepositerAddr := vars[RestDepositer] if len(strProposalID) == 0 { - w.WriteHeader(http.StatusBadRequest) err := errors.New("proposalId required but not specified") - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } @@ -225,19 +215,15 @@ func queryDepositHandlerFn(cdc *wire.Codec) http.HandlerFunc { } if len(bechDepositerAddr) == 0 { - w.WriteHeader(http.StatusBadRequest) err := errors.New("depositer address required but not specified") - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } depositerAddr, err := sdk.AccAddressFromBech32(bechDepositerAddr) if err != nil { - w.WriteHeader(http.StatusBadRequest) err := errors.Errorf("'%s' needs to be bech32 encoded", RestDepositer) - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } @@ -250,16 +236,13 @@ func queryDepositHandlerFn(cdc *wire.Codec) http.HandlerFunc { bz, err := cdc.MarshalJSON(params) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } res, err := cliCtx.QueryWithData("custom/gov/deposit", bz) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } @@ -268,14 +251,12 @@ func queryDepositHandlerFn(cdc *wire.Codec) http.HandlerFunc { if deposit.Empty() { res, err := cliCtx.QueryWithData("custom/gov/proposal", cdc.MustMarshalBinary(gov.QueryProposalParams{params.ProposalID})) if err != nil || len(res) == 0 { - w.WriteHeader(http.StatusNotFound) err := errors.Errorf("proposalID [%d] does not exist", proposalID) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusNotFound, err.Error()) return } - w.WriteHeader(http.StatusNotFound) err = errors.Errorf("depositer [%s] did not deposit on proposalID [%d]", bechDepositerAddr, proposalID) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusNotFound, err.Error()) return } @@ -290,9 +271,8 @@ func queryVoteHandlerFn(cdc *wire.Codec) http.HandlerFunc { bechVoterAddr := vars[RestVoter] if len(strProposalID) == 0 { - w.WriteHeader(http.StatusBadRequest) err := errors.New("proposalId required but not specified") - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } @@ -302,18 +282,15 @@ func queryVoteHandlerFn(cdc *wire.Codec) http.HandlerFunc { } if len(bechVoterAddr) == 0 { - w.WriteHeader(http.StatusBadRequest) err := errors.New("voter address required but not specified") - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } voterAddr, err := sdk.AccAddressFromBech32(bechVoterAddr) if err != nil { - w.WriteHeader(http.StatusBadRequest) err := errors.Errorf("'%s' needs to be bech32 encoded", RestVoter) - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } @@ -325,16 +302,13 @@ func queryVoteHandlerFn(cdc *wire.Codec) http.HandlerFunc { } bz, err := cdc.MarshalJSON(params) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } res, err := cliCtx.QueryWithData("custom/gov/vote", bz) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } @@ -343,20 +317,17 @@ func queryVoteHandlerFn(cdc *wire.Codec) http.HandlerFunc { if vote.Empty() { bz, err := cdc.MarshalJSON(gov.QueryProposalParams{params.ProposalID}) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } res, err := cliCtx.QueryWithData("custom/gov/proposal", bz) if err != nil || len(res) == 0 { - w.WriteHeader(http.StatusNotFound) err := errors.Errorf("proposalID [%d] does not exist", proposalID) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusNotFound, err.Error()) return } - w.WriteHeader(http.StatusNotFound) err = errors.Errorf("voter [%s] did not deposit on proposalID [%d]", bechVoterAddr, proposalID) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusNotFound, err.Error()) return } w.Write(res) @@ -371,10 +342,8 @@ func queryVotesOnProposalHandlerFn(cdc *wire.Codec) http.HandlerFunc { strProposalID := vars[RestProposalID] if len(strProposalID) == 0 { - w.WriteHeader(http.StatusBadRequest) err := errors.New("proposalId required but not specified") - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } @@ -390,15 +359,13 @@ func queryVotesOnProposalHandlerFn(cdc *wire.Codec) http.HandlerFunc { } bz, err := cdc.MarshalJSON(params) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } res, err := cliCtx.QueryWithData("custom/gov/votes", bz) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } @@ -420,9 +387,8 @@ func queryProposalsWithParameterFn(cdc *wire.Codec) http.HandlerFunc { if len(bechVoterAddr) != 0 { voterAddr, err := sdk.AccAddressFromBech32(bechVoterAddr) if err != nil { - w.WriteHeader(http.StatusBadRequest) err := errors.Errorf("'%s' needs to be bech32 encoded", RestVoter) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } params.Voter = voterAddr @@ -431,10 +397,8 @@ func queryProposalsWithParameterFn(cdc *wire.Codec) http.HandlerFunc { if len(bechDepositerAddr) != 0 { depositerAddr, err := sdk.AccAddressFromBech32(bechDepositerAddr) if err != nil { - w.WriteHeader(http.StatusBadRequest) err := errors.Errorf("'%s' needs to be bech32 encoded", RestDepositer) - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } params.Depositer = depositerAddr @@ -443,10 +407,8 @@ func queryProposalsWithParameterFn(cdc *wire.Codec) http.HandlerFunc { if len(strProposalStatus) != 0 { proposalStatus, err := gov.ProposalStatusFromString(strProposalStatus) if err != nil { - w.WriteHeader(http.StatusBadRequest) err := errors.Errorf("'%s' is not a valid Proposal Status", strProposalStatus) - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } params.ProposalStatus = proposalStatus @@ -461,8 +423,7 @@ func queryProposalsWithParameterFn(cdc *wire.Codec) http.HandlerFunc { bz, err := cdc.MarshalJSON(params) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } @@ -470,9 +431,7 @@ func queryProposalsWithParameterFn(cdc *wire.Codec) http.HandlerFunc { res, err := cliCtx.QueryWithData("custom/gov/proposals", bz) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } diff --git a/x/gov/client/rest/util.go b/x/gov/client/rest/util.go index e6aa83cc4..c6b360dc8 100644 --- a/x/gov/client/rest/util.go +++ b/x/gov/client/rest/util.go @@ -7,11 +7,10 @@ import ( "strconv" "github.com/cosmos/cosmos-sdk/client/context" + "github.com/cosmos/cosmos-sdk/client/utils" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/wire" authctx "github.com/cosmos/cosmos-sdk/x/auth/client/context" - - "github.com/pkg/errors" ) type baseReq struct { @@ -26,12 +25,12 @@ type baseReq struct { func buildReq(w http.ResponseWriter, r *http.Request, cdc *wire.Codec, req interface{}) error { body, err := ioutil.ReadAll(r.Body) if err != nil { - writeErr(&w, http.StatusBadRequest, err.Error()) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return err } err = cdc.UnmarshalJSON(body, req) if err != nil { - writeErr(&w, http.StatusBadRequest, err.Error()) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return err } return nil @@ -39,41 +38,36 @@ func buildReq(w http.ResponseWriter, r *http.Request, cdc *wire.Codec, req inter func (req baseReq) baseReqValidate(w http.ResponseWriter) bool { if len(req.Name) == 0 { - writeErr(&w, http.StatusUnauthorized, "Name required but not specified") + utils.WriteErrorResponse(&w, http.StatusUnauthorized, "Name required but not specified") return false } if len(req.Password) == 0 { - writeErr(&w, http.StatusUnauthorized, "Password required but not specified") + utils.WriteErrorResponse(&w, http.StatusUnauthorized, "Password required but not specified") return false } if len(req.ChainID) == 0 { - writeErr(&w, http.StatusUnauthorized, "ChainID required but not specified") + utils.WriteErrorResponse(&w, http.StatusUnauthorized, "ChainID required but not specified") return false } if req.AccountNumber < 0 { - writeErr(&w, http.StatusUnauthorized, "Account Number required but not specified") + utils.WriteErrorResponse(&w, http.StatusUnauthorized, "Account Number required but not specified") return false } if req.Sequence < 0 { - writeErr(&w, http.StatusUnauthorized, "Sequence required but not specified") + utils.WriteErrorResponse(&w, http.StatusUnauthorized, "Sequence required but not specified") return false } return true } -func writeErr(w *http.ResponseWriter, status int, msg string) { - (*w).WriteHeader(status) - err := errors.New(msg) - (*w).Write([]byte(err.Error())) -} - // TODO: Build this function out into a more generic base-request // (probably should live in client/lcd). func signAndBuild(w http.ResponseWriter, cliCtx context.CLIContext, baseReq baseReq, msg sdk.Msg, cdc *wire.Codec) { + var err error txCtx := authctx.TxContext{ Codec: cdc, AccountNumber: baseReq.AccountNumber, @@ -82,21 +76,28 @@ func signAndBuild(w http.ResponseWriter, cliCtx context.CLIContext, baseReq base Gas: baseReq.Gas, } + if baseReq.Gas == 0 { + txCtx, err = utils.EnrichTxContextWithGas(txCtx, cliCtx, baseReq.Name, baseReq.Password, []sdk.Msg{msg}) + if err != nil { + utils.WriteErrorResponse(&w, http.StatusUnauthorized, err.Error()) + return + } + } txBytes, err := txCtx.BuildAndSign(baseReq.Name, baseReq.Password, []sdk.Msg{msg}) if err != nil { - writeErr(&w, http.StatusUnauthorized, err.Error()) + utils.WriteErrorResponse(&w, http.StatusUnauthorized, err.Error()) return } res, err := cliCtx.BroadcastTx(txBytes) if err != nil { - writeErr(&w, http.StatusInternalServerError, err.Error()) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } output, err := wire.MarshalJSONIndent(cdc, res) if err != nil { - writeErr(&w, http.StatusInternalServerError, err.Error()) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } diff --git a/x/ibc/client/rest/transfer.go b/x/ibc/client/rest/transfer.go index 4470f556e..8c12a4b82 100644 --- a/x/ibc/client/rest/transfer.go +++ b/x/ibc/client/rest/transfer.go @@ -5,6 +5,7 @@ import ( "net/http" "github.com/cosmos/cosmos-sdk/client/context" + "github.com/cosmos/cosmos-sdk/client/utils" "github.com/cosmos/cosmos-sdk/crypto/keys" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/wire" @@ -40,30 +41,26 @@ func TransferRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx context.C to, err := sdk.AccAddressFromBech32(bech32addr) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } var m transferBody body, err := ioutil.ReadAll(r.Body) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } err = cdc.UnmarshalJSON(body, &m) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } info, err := kb.Get(m.LocalAccountName) if err != nil { - w.WriteHeader(http.StatusUnauthorized) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusUnauthorized, err.Error()) return } @@ -79,24 +76,30 @@ func TransferRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx context.C Gas: m.Gas, } + if m.Gas == 0 { + txCtx, err = utils.EnrichTxContextWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, []sdk.Msg{msg}) + if err != nil { + w.WriteHeader(http.StatusUnauthorized) + w.Write([]byte(err.Error())) + return + } + } + txBytes, err := txCtx.BuildAndSign(m.LocalAccountName, m.Password, []sdk.Msg{msg}) if err != nil { - w.WriteHeader(http.StatusUnauthorized) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusUnauthorized, err.Error()) return } res, err := cliCtx.BroadcastTx(txBytes) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } output, err := cdc.MarshalJSON(res) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } diff --git a/x/slashing/client/rest/tx.go b/x/slashing/client/rest/tx.go index 6e45230b0..956e7a571 100644 --- a/x/slashing/client/rest/tx.go +++ b/x/slashing/client/rest/tx.go @@ -8,6 +8,7 @@ import ( "net/http" "github.com/cosmos/cosmos-sdk/client/context" + "github.com/cosmos/cosmos-sdk/client/utils" "github.com/cosmos/cosmos-sdk/crypto/keys" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/wire" @@ -40,34 +41,29 @@ func unjailRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx context.CLI var m UnjailBody body, err := ioutil.ReadAll(r.Body) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } err = json.Unmarshal(body, &m) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusBadRequest, err.Error()) return } info, err := kb.Get(m.LocalAccountName) if err != nil { - w.WriteHeader(http.StatusUnauthorized) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusUnauthorized, err.Error()) return } validatorAddr, err := sdk.AccAddressFromBech32(m.ValidatorAddr) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error()))) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error())) return } if !bytes.Equal(info.GetPubKey().Address(), validatorAddr) { - w.WriteHeader(http.StatusUnauthorized) - w.Write([]byte("Must use own validator address")) + utils.WriteErrorResponse(&w, http.StatusUnauthorized, "Must use own validator address") return } @@ -81,24 +77,29 @@ func unjailRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx context.CLI msg := slashing.NewMsgUnjail(validatorAddr) + if m.Gas == 0 { + txCtx, err = utils.EnrichTxContextWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, []sdk.Msg{msg}) + if err != nil { + utils.WriteErrorResponse(&w, http.StatusUnauthorized, err.Error()) + return + } + } + txBytes, err := txCtx.BuildAndSign(m.LocalAccountName, m.Password, []sdk.Msg{msg}) if err != nil { - w.WriteHeader(http.StatusUnauthorized) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusUnauthorized, "Must use own validator address") return } res, err := cliCtx.BroadcastTx(txBytes) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } output, err := json.MarshalIndent(res, "", " ") if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } diff --git a/x/stake/client/rest/tx.go b/x/stake/client/rest/tx.go index d8b9b6011..e049212d8 100644 --- a/x/stake/client/rest/tx.go +++ b/x/stake/client/rest/tx.go @@ -7,6 +7,7 @@ import ( "net/http" "github.com/cosmos/cosmos-sdk/client/context" + "github.com/cosmos/cosmos-sdk/client/utils" "github.com/cosmos/cosmos-sdk/crypto/keys" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/wire" @@ -105,21 +106,18 @@ func delegationsRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx contex for _, msg := range m.Delegations { delegatorAddr, err := sdk.AccAddressFromBech32(msg.DelegatorAddr) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("Couldn't decode delegator. Error: %s", err.Error()))) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode delegator. Error: %s", err.Error())) return } validatorAddr, err := sdk.AccAddressFromBech32(msg.ValidatorAddr) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error()))) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error())) return } if !bytes.Equal(info.GetPubKey().Address(), delegatorAddr) { - w.WriteHeader(http.StatusUnauthorized) - w.Write([]byte("Must use own delegator address")) + utils.WriteErrorResponse(&w, http.StatusUnauthorized, "Must use own delegator address") return } @@ -135,34 +133,29 @@ func delegationsRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx contex for _, msg := range m.BeginRedelegates { delegatorAddr, err := sdk.AccAddressFromBech32(msg.DelegatorAddr) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("Couldn't decode delegator. Error: %s", err.Error()))) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error())) return } if !bytes.Equal(info.GetPubKey().Address(), delegatorAddr) { - w.WriteHeader(http.StatusUnauthorized) - w.Write([]byte("Must use own delegator address")) + utils.WriteErrorResponse(&w, http.StatusUnauthorized, "Must use own delegator address") return } validatorSrcAddr, err := sdk.AccAddressFromBech32(msg.ValidatorSrcAddr) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error()))) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error())) return } validatorDstAddr, err := sdk.AccAddressFromBech32(msg.ValidatorDstAddr) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error()))) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error())) return } shares, err := sdk.NewDecFromStr(msg.SharesAmount) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("Couldn't decode shares amount. Error: %s", err.Error()))) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode shares amount. Error: %s", err.Error())) return } @@ -179,27 +172,23 @@ func delegationsRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx contex for _, msg := range m.CompleteRedelegates { delegatorAddr, err := sdk.AccAddressFromBech32(msg.DelegatorAddr) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("Couldn't decode delegator. Error: %s", err.Error()))) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode delegator. Error: %s", err.Error())) return } validatorSrcAddr, err := sdk.AccAddressFromBech32(msg.ValidatorSrcAddr) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error()))) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error())) return } validatorDstAddr, err := sdk.AccAddressFromBech32(msg.ValidatorDstAddr) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error()))) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error())) return } if !bytes.Equal(info.GetPubKey().Address(), delegatorAddr) { - w.WriteHeader(http.StatusUnauthorized) - w.Write([]byte("Must use own delegator address")) + utils.WriteErrorResponse(&w, http.StatusUnauthorized, "Must use own delegator address") return } @@ -215,28 +204,24 @@ func delegationsRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx contex for _, msg := range m.BeginUnbondings { delegatorAddr, err := sdk.AccAddressFromBech32(msg.DelegatorAddr) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("Couldn't decode delegator. Error: %s", err.Error()))) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode delegator. Error: %s", err.Error())) return } if !bytes.Equal(info.GetPubKey().Address(), delegatorAddr) { - w.WriteHeader(http.StatusUnauthorized) - w.Write([]byte("Must use own delegator address")) + utils.WriteErrorResponse(&w, http.StatusUnauthorized, "Must use own delegator address") return } validatorAddr, err := sdk.AccAddressFromBech32(msg.ValidatorAddr) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error()))) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error())) return } shares, err := sdk.NewDecFromStr(msg.SharesAmount) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("Couldn't decode shares amount. Error: %s", err.Error()))) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode shares amount. Error: %s", err.Error())) return } @@ -252,21 +237,18 @@ func delegationsRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx contex for _, msg := range m.CompleteUnbondings { delegatorAddr, err := sdk.AccAddressFromBech32(msg.DelegatorAddr) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("Couldn't decode delegator. Error: %s", err.Error()))) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode delegator. Error: %s", err.Error())) return } validatorAddr, err := sdk.AccAddressFromBech32(msg.ValidatorAddr) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error()))) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error())) return } if !bytes.Equal(info.GetPubKey().Address(), delegatorAddr) { - w.WriteHeader(http.StatusUnauthorized) - w.Write([]byte("Must use own delegator address")) + utils.WriteErrorResponse(&w, http.StatusUnauthorized, "Must use own delegator address") return } @@ -293,10 +275,17 @@ func delegationsRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx contex m.Sequence++ + if m.Gas == 0 { + txCtx, err = utils.EnrichTxContextWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, []sdk.Msg{msg}) + if err != nil { + utils.WriteErrorResponse(&w, http.StatusUnauthorized, err.Error()) + return + } + } + txBytes, err := txCtx.BuildAndSign(m.LocalAccountName, m.Password, []sdk.Msg{msg}) if err != nil { - w.WriteHeader(http.StatusUnauthorized) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusUnauthorized, err.Error()) return } @@ -310,8 +299,7 @@ func delegationsRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx contex for i, txBytes := range signedTxs { res, err := cliCtx.BroadcastTx(txBytes) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } @@ -320,8 +308,7 @@ func delegationsRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx contex output, err := wire.MarshalJSONIndent(cdc, results[:]) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } From 323fac3bf7791acbaff51fb7a69eb0f9837014cb Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Wed, 22 Aug 2018 12:40:07 +0100 Subject: [PATCH 04/25] Update PENDING.md --- PENDING.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/PENDING.md b/PENDING.md index 01bdc2158..d1f87da5a 100644 --- a/PENDING.md +++ b/PENDING.md @@ -35,6 +35,8 @@ FEATURES * Gaia CLI (`gaiacli`) * [cli] Cmds to query staking pool and params * [gov][cli] #2062 added `--proposal` flag to `submit-proposal` that allows a JSON file containing a proposal to be passed in + * [cli] \#2047 Setting the --gas flag value to 0 triggers a simulation of the tx before the actual execution. The gas estimate obtained via the simulation will be used as gas limit in the actual execution. + * [cli] \#2047 The --gas-adjustment flag can be used to adjust the estimate obtained via the simulation triggered by --gas=0. * Gaia From 47d55bd57217fb2adaedb6ada42b0edeb16d7b71 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Thu, 23 Aug 2018 16:10:33 +0100 Subject: [PATCH 05/25] Add/refresh docs --- docs/sdk/clients.md | 6 ++++++ docs/sdk/core/app1.md | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/sdk/clients.md b/docs/sdk/clients.md index be42a46db..0a1c2a38d 100644 --- a/docs/sdk/clients.md +++ b/docs/sdk/clients.md @@ -97,6 +97,12 @@ gaiacli send \ The `--amount` flag accepts the format `--amount=`. ::: +::: tip Note +You may want to cap the maximum gas that can be consumed by the transaction via the `--gas` flag. +If set to 0, the gas limit will be automatically estimated. +Gas estimate might be inaccurate as state changes could occur in between the end of the simulation and the actual execution of a transaction, thus an adjustment is applied on top of the original estimate in order to ensure the transaction is broadcasted successfully. The adjustment can be controlled via the `--gas-adjustment` flag, whose default value is 1.2. +::: + Now, view the updated balances of the origin and destination accounts: ```bash diff --git a/docs/sdk/core/app1.md b/docs/sdk/core/app1.md index a2978ffb0..9338d30d9 100644 --- a/docs/sdk/core/app1.md +++ b/docs/sdk/core/app1.md @@ -208,7 +208,7 @@ type Result struct { // GasWanted is the maximum units of work we allow this tx to perform. GasWanted int64 - // GasUsed is the amount of gas actually consumed. NOTE: unimplemented + // GasUsed is the amount of gas actually consumed. GasUsed int64 // Tx fee amount and denom. From e959478e61697a446762bf7863bb0fff438f6eca Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Fri, 24 Aug 2018 07:34:24 +0100 Subject: [PATCH 06/25] comment getContextForAnte(), rename applyTxMode() --- baseapp/baseapp.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 2669aa9f3..2a52b7179 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -493,6 +493,8 @@ func validateBasicTxMsgs(msgs []sdk.Msg) sdk.Error { return nil } +// retrieve the context for the ante handler and store the tx bytes; store +// the signing validators 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) @@ -565,7 +567,7 @@ func getState(app *BaseApp, mode runTxMode) *state { return app.deliverState } -func (app *BaseApp) applyTxMode(ctx sdk.Context, mode runTxMode) sdk.Context { +func (app *BaseApp) initializeContext(ctx sdk.Context, mode runTxMode) sdk.Context { if mode != runTxModeSimulate { return ctx } @@ -582,7 +584,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk var gasWanted int64 var msCache sdk.CacheMultiStore ctx := app.getContextForAnte(mode, txBytes) - ctx = app.applyTxMode(ctx, mode) + ctx = app.initializeContext(ctx, mode) defer func() { if r := recover(); r != nil { From 7e9ceb452db5d1f0256af66004f6f0a357863f19 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Fri, 24 Aug 2018 07:35:12 +0100 Subject: [PATCH 07/25] clarifyy that adjustment is a multiplicative factor --- client/flags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/flags.go b/client/flags.go index d128d32f6..15cc9effa 100644 --- a/client/flags.go +++ b/client/flags.go @@ -53,7 +53,7 @@ func PostCommands(cmds ...*cobra.Command) []*cobra.Command { c.Flags().String(FlagNode, "tcp://localhost:26657", ": to tendermint rpc interface for this chain") c.Flags().Bool(FlagUseLedger, false, "Use a connected Ledger device") c.Flags().Int64(FlagGas, 0, "gas limit to set per-transaction; set to 0 to calculate required gas automatically") - c.Flags().Float64(FlagGasAdjustment, DefaultGasAdjustment, "gas adjustment to be applied on the estimate returned by the tx simulation") + c.Flags().Float64(FlagGasAdjustment, DefaultGasAdjustment, "adjustment factor to be multiplied against the estimate returned by the tx simulation; defaults to an internal value") c.Flags().Bool(FlagAsync, false, "broadcast transactions asynchronously") c.Flags().Bool(FlagJson, false, "return output in json format") c.Flags().Bool(FlagPrintResponse, true, "return tx response (only works with async = false)") From 2dea46779c562d6af21bedd62866aed9c8813569 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Fri, 24 Aug 2018 07:37:41 +0100 Subject: [PATCH 08/25] TestCoinSend: test success case when setting gas by hand --- client/lcd/lcd_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/client/lcd/lcd_test.go b/client/lcd/lcd_test.go index 2db2f6d61..67d870883 100644 --- a/client/lcd/lcd_test.go +++ b/client/lcd/lcd_test.go @@ -267,6 +267,10 @@ func TestCoinSend(t *testing.T) { // test failure with too little gas res, body, _ = doSendWithGas(t, port, seed, name, password, addr, 100) require.Equal(t, http.StatusInternalServerError, res.StatusCode, body) + + // test success with just enough gas + res, body, _ = doSendWithGas(t, port, seed, name, password, addr, 3000) + require.Equal(t, http.StatusOK, res.StatusCode, body) } func TestIBCTransfer(t *testing.T) { From fb5fe9914d2489f15450a0b9ccde9b5c06a26e16 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Fri, 24 Aug 2018 07:54:25 +0100 Subject: [PATCH 09/25] simplify json handling in LCD tests --- client/lcd/lcd_test.go | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/client/lcd/lcd_test.go b/client/lcd/lcd_test.go index 67d870883..298cf0027 100644 --- a/client/lcd/lcd_test.go +++ b/client/lcd/lcd_test.go @@ -738,27 +738,22 @@ func doSendWithGas(t *testing.T, port, seed, name, password string, addr sdk.Acc panic(err) } - jsonStr := func() []byte { - if gas > 0 { - return []byte(fmt.Sprintf(`{ - "name":"%s", - "password":"%s", - "account_number":"%d", - "sequence":"%d", - "amount":[%s], - "chain_id":"%s", - "gas":"%v" - }`, name, password, accnum, sequence, coinbz, chainID, gas)) - } - return []byte(fmt.Sprintf(`{ - "name":"%s", - "password":"%s", - "account_number":"%d", - "sequence":"%d", - "amount":[%s], - "chain_id":"%s" - }`, name, password, accnum, sequence, coinbz, chainID)) - }() + gasStr := "" + if gas > 0 { + gasStr = fmt.Sprintf(` + "gas":"%v", + `, gas) + } + jsonStr := []byte(fmt.Sprintf(`{ + %v + "name":"%s", + "password":"%s", + "account_number":"%d", + "sequence":"%d", + "amount":[%s], + "chain_id":"%s" + }`, gasStr, name, password, accnum, sequence, coinbz, chainID)) + res, body = Request(t, port, "POST", fmt.Sprintf("/accounts/%s/send", receiveAddr), jsonStr) return } From f36f749818e1f2d08cddbe52cc9c3a62a2b8c202 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Fri, 24 Aug 2018 09:48:02 +0100 Subject: [PATCH 10/25] Incorporating @ValarDragon's comments --- client/utils/utils.go | 45 +++++++++++++++++++++-------------- x/bank/client/rest/sendtx.go | 17 +++++++++++-- x/gov/client/rest/util.go | 17 +++++++++++-- x/ibc/client/rest/transfer.go | 18 +++++++++++--- x/slashing/client/rest/tx.go | 17 +++++++++++-- x/stake/client/rest/tx.go | 17 +++++++++++-- 6 files changed, 102 insertions(+), 29 deletions(-) diff --git a/client/utils/utils.go b/client/utils/utils.go index 9f06dda1d..da49ff5e0 100644 --- a/client/utils/utils.go +++ b/client/utils/utils.go @@ -9,6 +9,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" authctx "github.com/cosmos/cosmos-sdk/x/auth/client/context" amino "github.com/tendermint/go-amino" + "github.com/tendermint/tendermint/libs/common" ) // DefaultGasAdjustment is applied to gas estimates to avoid tx @@ -58,7 +59,7 @@ func SendTx(txCtx authctx.TxContext, cliCtx context.CLIContext, msgs []sdk.Msg) return err } - txCtx, err = enrichCtxWithGasIfGasAuto(txCtx, cliCtx, cliCtx.FromAddressName, passphrase, msgs) + txCtx, err = enrichCtxWithGasIfGasAuto(txCtx, cliCtx, passphrase, msgs) if err != nil { return err } @@ -72,35 +73,43 @@ func SendTx(txCtx authctx.TxContext, cliCtx context.CLIContext, msgs []sdk.Msg) return cliCtx.EnsureBroadcastTx(txBytes) } -func enrichCtxWithGasIfGasAuto(txCtx authctx.TxContext, cliCtx context.CLIContext, name, passphrase string, msgs []sdk.Msg) (authctx.TxContext, error) { +func enrichCtxWithGasIfGasAuto(txCtx authctx.TxContext, cliCtx context.CLIContext, passphrase string, msgs []sdk.Msg) (authctx.TxContext, error) { if cliCtx.Gas == 0 { - return EnrichTxContextWithGas(txCtx, cliCtx, name, passphrase, msgs) + txBytes, err := BuildAndSignTxWithZeroGas(txCtx, cliCtx.FromAddressName, passphrase, msgs) + if err != nil { + return txCtx, err + } + estimate, adjusted, err := CalculateGas(cliCtx.Query, cliCtx.Codec, txBytes, cliCtx.GasAdjustment) + if err != nil { + return txCtx, err + } + fmt.Fprintf(os.Stderr, "gas: [estimated = %v] [adjusted = %v]\n", estimate, adjusted) + return txCtx.WithGas(adjusted), nil } return txCtx, nil } -// EnrichTxContextWithGas simulates the execution of a transaction to -// then populate the relevant TxContext.Gas field with the estimate -// obtained by the query. -func EnrichTxContextWithGas(txCtx authctx.TxContext, cliCtx context.CLIContext, name, passphrase string, msgs []sdk.Msg) (authctx.TxContext, error) { - txCtxSimulation := txCtx.WithGas(0) - txBytes, err := txCtxSimulation.BuildAndSign(name, passphrase, msgs) - if err != nil { - return txCtx, err - } +// BuildAndSignTxWithZeroGas builds transactions with GasWanted set to 0. +func BuildAndSignTxWithZeroGas(txCtx authctx.TxContext, name, passphrase string, msgs []sdk.Msg) ([]byte, error) { + return txCtx.WithGas(0).BuildAndSign(name, passphrase, msgs) +} + +// 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) { // run a simulation (via /app/simulate query) to // estimate gas and update TxContext accordingly - rawRes, err := cliCtx.Query("/app/simulate", txBytes) + rawRes, err := queryFunc("/app/simulate", txBytes) if err != nil { - return txCtx, err + return } - estimate, err := parseQueryResponse(cliCtx.Codec, rawRes) + estimate, err = parseQueryResponse(cdc, rawRes) if err != nil { - return txCtx, err + return } - adjusted := adjustGasEstimate(estimate, cliCtx.GasAdjustment) + adjusted = adjustGasEstimate(estimate, adjustment) fmt.Fprintf(os.Stderr, "gas: [estimated = %v] [adjusted = %v]\n", estimate, adjusted) - return txCtx.WithGas(adjusted), nil + return } func adjustGasEstimate(estimate int64, adjustment float64) int64 { diff --git a/x/bank/client/rest/sendtx.go b/x/bank/client/rest/sendtx.go index d27be38b5..14bff9a49 100644 --- a/x/bank/client/rest/sendtx.go +++ b/x/bank/client/rest/sendtx.go @@ -86,11 +86,12 @@ func SendRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx context.CLICo } if m.Gas == 0 { - txCtx, err = utils.EnrichTxContextWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, []sdk.Msg{msg}) + newCtx, httperr, err := enrichContextWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, msg) if err != nil { - utils.WriteErrorResponse(&w, http.StatusUnauthorized, err.Error()) + utils.WriteErrorResponse(&w, httperr, err.Error()) return } + txCtx = newCtx } txBytes, err := txCtx.BuildAndSign(m.LocalAccountName, m.Password, []sdk.Msg{msg}) @@ -114,3 +115,15 @@ func SendRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx context.CLICo w.Write(output) } } + +func enrichContextWithGas(txCtx authctx.TxContext, cliCtx context.CLIContext, name, password string, msg sdk.Msg) (authctx.TxContext, int, error) { + txBytes, err := utils.BuildAndSignTxWithZeroGas(txCtx, name, password, []sdk.Msg{msg}) + if err != nil { + return txCtx, http.StatusInternalServerError, err + } + _, adjusted, err := utils.CalculateGas(cliCtx.Query, cliCtx.Codec, txBytes, cliCtx.GasAdjustment) + if err != nil { + return txCtx, http.StatusUnauthorized, err + } + return txCtx.WithGas(adjusted), http.StatusOK, nil +} diff --git a/x/gov/client/rest/util.go b/x/gov/client/rest/util.go index c6b360dc8..e20402831 100644 --- a/x/gov/client/rest/util.go +++ b/x/gov/client/rest/util.go @@ -77,11 +77,12 @@ func signAndBuild(w http.ResponseWriter, cliCtx context.CLIContext, baseReq base } if baseReq.Gas == 0 { - txCtx, err = utils.EnrichTxContextWithGas(txCtx, cliCtx, baseReq.Name, baseReq.Password, []sdk.Msg{msg}) + newCtx, httperr, err := enrichContextWithGas(txCtx, cliCtx, baseReq.Name, baseReq.Password, msg) if err != nil { - utils.WriteErrorResponse(&w, http.StatusUnauthorized, err.Error()) + utils.WriteErrorResponse(&w, httperr, err.Error()) return } + txCtx = newCtx } txBytes, err := txCtx.BuildAndSign(baseReq.Name, baseReq.Password, []sdk.Msg{msg}) if err != nil { @@ -115,3 +116,15 @@ func parseInt64OrReturnBadRequest(s string, w http.ResponseWriter) (n int64, ok } return n, true } + +func enrichContextWithGas(txCtx authctx.TxContext, cliCtx context.CLIContext, name, password string, msg sdk.Msg) (authctx.TxContext, int, error) { + txBytes, err := utils.BuildAndSignTxWithZeroGas(txCtx, name, password, []sdk.Msg{msg}) + if err != nil { + return txCtx, http.StatusInternalServerError, err + } + _, adjusted, err := utils.CalculateGas(cliCtx.Query, cliCtx.Codec, txBytes, cliCtx.GasAdjustment) + if err != nil { + return txCtx, http.StatusUnauthorized, err + } + return txCtx.WithGas(adjusted), http.StatusOK, nil +} diff --git a/x/ibc/client/rest/transfer.go b/x/ibc/client/rest/transfer.go index 8c12a4b82..cf0f24b31 100644 --- a/x/ibc/client/rest/transfer.go +++ b/x/ibc/client/rest/transfer.go @@ -77,12 +77,12 @@ func TransferRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx context.C } if m.Gas == 0 { - txCtx, err = utils.EnrichTxContextWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, []sdk.Msg{msg}) + newCtx, httperr, err := enrichContextWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, msg) if err != nil { - w.WriteHeader(http.StatusUnauthorized) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(&w, httperr, err.Error()) return } + txCtx = newCtx } txBytes, err := txCtx.BuildAndSign(m.LocalAccountName, m.Password, []sdk.Msg{msg}) @@ -106,3 +106,15 @@ func TransferRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx context.C w.Write(output) } } + +func enrichContextWithGas(txCtx authctx.TxContext, cliCtx context.CLIContext, name, password string, msg sdk.Msg) (authctx.TxContext, int, error) { + txBytes, err := utils.BuildAndSignTxWithZeroGas(txCtx, name, password, []sdk.Msg{msg}) + if err != nil { + return txCtx, http.StatusInternalServerError, err + } + _, adjusted, err := utils.CalculateGas(cliCtx.Query, cliCtx.Codec, txBytes, cliCtx.GasAdjustment) + if err != nil { + return txCtx, http.StatusUnauthorized, err + } + return txCtx.WithGas(adjusted), http.StatusOK, nil +} diff --git a/x/slashing/client/rest/tx.go b/x/slashing/client/rest/tx.go index 956e7a571..414b05cfd 100644 --- a/x/slashing/client/rest/tx.go +++ b/x/slashing/client/rest/tx.go @@ -78,11 +78,12 @@ func unjailRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx context.CLI msg := slashing.NewMsgUnjail(validatorAddr) if m.Gas == 0 { - txCtx, err = utils.EnrichTxContextWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, []sdk.Msg{msg}) + newCtx, httperr, err := enrichContextWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, msg) if err != nil { - utils.WriteErrorResponse(&w, http.StatusUnauthorized, err.Error()) + utils.WriteErrorResponse(&w, httperr, err.Error()) return } + txCtx = newCtx } txBytes, err := txCtx.BuildAndSign(m.LocalAccountName, m.Password, []sdk.Msg{msg}) @@ -106,3 +107,15 @@ func unjailRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx context.CLI w.Write(output) } } + +func enrichContextWithGas(txCtx authctx.TxContext, cliCtx context.CLIContext, name, password string, msg sdk.Msg) (authctx.TxContext, int, error) { + txBytes, err := utils.BuildAndSignTxWithZeroGas(txCtx, name, password, []sdk.Msg{msg}) + if err != nil { + return txCtx, http.StatusInternalServerError, err + } + _, adjusted, err := utils.CalculateGas(cliCtx.Query, cliCtx.Codec, txBytes, cliCtx.GasAdjustment) + if err != nil { + return txCtx, http.StatusUnauthorized, err + } + return txCtx.WithGas(adjusted), http.StatusOK, nil +} diff --git a/x/stake/client/rest/tx.go b/x/stake/client/rest/tx.go index e049212d8..729f57895 100644 --- a/x/stake/client/rest/tx.go +++ b/x/stake/client/rest/tx.go @@ -276,11 +276,12 @@ func delegationsRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx contex m.Sequence++ if m.Gas == 0 { - txCtx, err = utils.EnrichTxContextWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, []sdk.Msg{msg}) + newCtx, httperr, err := enrichContextWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, msg) if err != nil { - utils.WriteErrorResponse(&w, http.StatusUnauthorized, err.Error()) + utils.WriteErrorResponse(&w, httperr, err.Error()) return } + txCtx = newCtx } txBytes, err := txCtx.BuildAndSign(m.LocalAccountName, m.Password, []sdk.Msg{msg}) @@ -315,3 +316,15 @@ func delegationsRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx contex w.Write(output) } } + +func enrichContextWithGas(txCtx authcliCtx.TxContext, cliCtx context.CLIContext, name, password string, msg sdk.Msg) (authcliCtx.TxContext, int, error) { + txBytes, err := utils.BuildAndSignTxWithZeroGas(txCtx, name, password, []sdk.Msg{msg}) + if err != nil { + return txCtx, http.StatusInternalServerError, err + } + _, adjusted, err := utils.CalculateGas(cliCtx.Query, cliCtx.Codec, txBytes, cliCtx.GasAdjustment) + if err != nil { + return txCtx, http.StatusUnauthorized, err + } + return txCtx.WithGas(adjusted), http.StatusOK, nil +} From 7e8feec73833e3c793f6268b289f82778e48cb33 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Fri, 24 Aug 2018 14:57:45 +0100 Subject: [PATCH 11/25] Incorporating @cwgoes comments --- baseapp/baseapp.go | 11 +++++------ client/flags.go | 4 ++-- client/utils/utils.go | 33 +++++++++++++++++---------------- x/bank/client/rest/sendtx.go | 16 ++-------------- x/gov/client/rest/util.go | 16 ++-------------- x/ibc/client/rest/transfer.go | 16 ++-------------- x/slashing/client/rest/tx.go | 16 ++-------------- x/stake/client/rest/tx.go | 16 ++-------------- 8 files changed, 34 insertions(+), 94 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 2a52b7179..20faf06cd 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -498,10 +498,9 @@ func validateBasicTxMsgs(msgs []sdk.Msg) sdk.Error { func (app *BaseApp) getContextForAnte(mode runTxMode, txBytes []byte) (ctx sdk.Context) { // Get the context ctx = getState(app, mode).ctx.WithTxBytes(txBytes) - if mode != runTxModeDeliver { - return + if mode == runTxModeDeliver { + ctx = ctx.WithSigningValidators(app.signedValidators) } - ctx = ctx.WithSigningValidators(app.signedValidators) return } @@ -568,10 +567,10 @@ func getState(app *BaseApp, mode runTxMode) *state { } func (app *BaseApp) initializeContext(ctx sdk.Context, mode runTxMode) sdk.Context { - if mode != runTxModeSimulate { - return ctx + if mode == runTxModeSimulate { + ctx = ctx.WithMultiStore(getState(app, runTxModeSimulate).CacheMultiStore()) } - return ctx.WithMultiStore(getState(app, runTxModeSimulate).CacheMultiStore()) + return ctx } // runTx processes a transaction. The transactions is proccessed via an diff --git a/client/flags.go b/client/flags.go index 15cc9effa..7337e9741 100644 --- a/client/flags.go +++ b/client/flags.go @@ -4,7 +4,7 @@ import "github.com/spf13/cobra" // nolint const ( - DefaultGasAdjustment = 0 + DefaultGasAdjustment = 1.2 FlagUseLedger = "ledger" FlagChainID = "chain-id" @@ -53,7 +53,7 @@ func PostCommands(cmds ...*cobra.Command) []*cobra.Command { c.Flags().String(FlagNode, "tcp://localhost:26657", ": to tendermint rpc interface for this chain") c.Flags().Bool(FlagUseLedger, false, "Use a connected Ledger device") c.Flags().Int64(FlagGas, 0, "gas limit to set per-transaction; set to 0 to calculate required gas automatically") - c.Flags().Float64(FlagGasAdjustment, DefaultGasAdjustment, "adjustment factor to be multiplied against the estimate returned by the tx simulation; defaults to an internal value") + c.Flags().Float64(FlagGasAdjustment, DefaultGasAdjustment, "adjustment factor to be multiplied against the estimate returned by the tx simulation") c.Flags().Bool(FlagAsync, false, "broadcast transactions asynchronously") c.Flags().Bool(FlagJson, false, "return output in json format") c.Flags().Bool(FlagPrintResponse, true, "return tx response (only works with async = false)") diff --git a/client/utils/utils.go b/client/utils/utils.go index da49ff5e0..fb5d61988 100644 --- a/client/utils/utils.go +++ b/client/utils/utils.go @@ -59,9 +59,11 @@ func SendTx(txCtx authctx.TxContext, cliCtx context.CLIContext, msgs []sdk.Msg) return err } - txCtx, err = enrichCtxWithGasIfGasAuto(txCtx, cliCtx, passphrase, msgs) - if err != nil { - return err + if cliCtx.Gas == 0 { + txCtx, err = EnrichCtxWithGas(txCtx, cliCtx, cliCtx.FromAddressName, passphrase, msgs) + if err != nil { + return err + } } // build and sign the transaction @@ -73,20 +75,19 @@ func SendTx(txCtx authctx.TxContext, cliCtx context.CLIContext, msgs []sdk.Msg) return cliCtx.EnsureBroadcastTx(txBytes) } -func enrichCtxWithGasIfGasAuto(txCtx authctx.TxContext, cliCtx context.CLIContext, passphrase string, msgs []sdk.Msg) (authctx.TxContext, error) { - if cliCtx.Gas == 0 { - txBytes, err := BuildAndSignTxWithZeroGas(txCtx, cliCtx.FromAddressName, passphrase, msgs) - if err != nil { - return txCtx, err - } - estimate, adjusted, err := CalculateGas(cliCtx.Query, cliCtx.Codec, txBytes, cliCtx.GasAdjustment) - if err != nil { - return txCtx, err - } - fmt.Fprintf(os.Stderr, "gas: [estimated = %v] [adjusted = %v]\n", estimate, adjusted) - return txCtx.WithGas(adjusted), nil +// EnrichCtxWithGas calculates the gas estimate that would be consumed by the +// transaction and set the transaction's respective value accordingly. +func EnrichCtxWithGas(txCtx authctx.TxContext, cliCtx context.CLIContext, name, passphrase string, msgs []sdk.Msg) (authctx.TxContext, error) { + txBytes, err := BuildAndSignTxWithZeroGas(txCtx, name, passphrase, msgs) + if err != nil { + return txCtx, err } - return txCtx, nil + estimate, adjusted, err := CalculateGas(cliCtx.Query, cliCtx.Codec, txBytes, cliCtx.GasAdjustment) + if err != nil { + return txCtx, err + } + fmt.Fprintf(os.Stderr, "gas: [estimated = %v] [adjusted = %v]\n", estimate, adjusted) + return txCtx.WithGas(adjusted), nil } // BuildAndSignTxWithZeroGas builds transactions with GasWanted set to 0. diff --git a/x/bank/client/rest/sendtx.go b/x/bank/client/rest/sendtx.go index 14bff9a49..c7baa9691 100644 --- a/x/bank/client/rest/sendtx.go +++ b/x/bank/client/rest/sendtx.go @@ -86,9 +86,9 @@ func SendRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx context.CLICo } if m.Gas == 0 { - newCtx, httperr, err := enrichContextWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, msg) + newCtx, err := utils.EnrichCtxWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, []sdk.Msg{msg}) if err != nil { - utils.WriteErrorResponse(&w, httperr, err.Error()) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } txCtx = newCtx @@ -115,15 +115,3 @@ func SendRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx context.CLICo w.Write(output) } } - -func enrichContextWithGas(txCtx authctx.TxContext, cliCtx context.CLIContext, name, password string, msg sdk.Msg) (authctx.TxContext, int, error) { - txBytes, err := utils.BuildAndSignTxWithZeroGas(txCtx, name, password, []sdk.Msg{msg}) - if err != nil { - return txCtx, http.StatusInternalServerError, err - } - _, adjusted, err := utils.CalculateGas(cliCtx.Query, cliCtx.Codec, txBytes, cliCtx.GasAdjustment) - if err != nil { - return txCtx, http.StatusUnauthorized, err - } - return txCtx.WithGas(adjusted), http.StatusOK, nil -} diff --git a/x/gov/client/rest/util.go b/x/gov/client/rest/util.go index e20402831..f98f7bfa5 100644 --- a/x/gov/client/rest/util.go +++ b/x/gov/client/rest/util.go @@ -77,9 +77,9 @@ func signAndBuild(w http.ResponseWriter, cliCtx context.CLIContext, baseReq base } if baseReq.Gas == 0 { - newCtx, httperr, err := enrichContextWithGas(txCtx, cliCtx, baseReq.Name, baseReq.Password, msg) + newCtx, err := utils.EnrichCtxWithGas(txCtx, cliCtx, baseReq.Name, baseReq.Password, []sdk.Msg{msg}) if err != nil { - utils.WriteErrorResponse(&w, httperr, err.Error()) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } txCtx = newCtx @@ -116,15 +116,3 @@ func parseInt64OrReturnBadRequest(s string, w http.ResponseWriter) (n int64, ok } return n, true } - -func enrichContextWithGas(txCtx authctx.TxContext, cliCtx context.CLIContext, name, password string, msg sdk.Msg) (authctx.TxContext, int, error) { - txBytes, err := utils.BuildAndSignTxWithZeroGas(txCtx, name, password, []sdk.Msg{msg}) - if err != nil { - return txCtx, http.StatusInternalServerError, err - } - _, adjusted, err := utils.CalculateGas(cliCtx.Query, cliCtx.Codec, txBytes, cliCtx.GasAdjustment) - if err != nil { - return txCtx, http.StatusUnauthorized, err - } - return txCtx.WithGas(adjusted), http.StatusOK, nil -} diff --git a/x/ibc/client/rest/transfer.go b/x/ibc/client/rest/transfer.go index cf0f24b31..765208b05 100644 --- a/x/ibc/client/rest/transfer.go +++ b/x/ibc/client/rest/transfer.go @@ -77,9 +77,9 @@ func TransferRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx context.C } if m.Gas == 0 { - newCtx, httperr, err := enrichContextWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, msg) + newCtx, err := utils.EnrichCtxWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, []sdk.Msg{msg}) if err != nil { - utils.WriteErrorResponse(&w, httperr, err.Error()) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } txCtx = newCtx @@ -106,15 +106,3 @@ func TransferRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx context.C w.Write(output) } } - -func enrichContextWithGas(txCtx authctx.TxContext, cliCtx context.CLIContext, name, password string, msg sdk.Msg) (authctx.TxContext, int, error) { - txBytes, err := utils.BuildAndSignTxWithZeroGas(txCtx, name, password, []sdk.Msg{msg}) - if err != nil { - return txCtx, http.StatusInternalServerError, err - } - _, adjusted, err := utils.CalculateGas(cliCtx.Query, cliCtx.Codec, txBytes, cliCtx.GasAdjustment) - if err != nil { - return txCtx, http.StatusUnauthorized, err - } - return txCtx.WithGas(adjusted), http.StatusOK, nil -} diff --git a/x/slashing/client/rest/tx.go b/x/slashing/client/rest/tx.go index 414b05cfd..26412ebc0 100644 --- a/x/slashing/client/rest/tx.go +++ b/x/slashing/client/rest/tx.go @@ -78,9 +78,9 @@ func unjailRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx context.CLI msg := slashing.NewMsgUnjail(validatorAddr) if m.Gas == 0 { - newCtx, httperr, err := enrichContextWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, msg) + newCtx, err := utils.EnrichCtxWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, []sdk.Msg{msg}) if err != nil { - utils.WriteErrorResponse(&w, httperr, err.Error()) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } txCtx = newCtx @@ -107,15 +107,3 @@ func unjailRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx context.CLI w.Write(output) } } - -func enrichContextWithGas(txCtx authctx.TxContext, cliCtx context.CLIContext, name, password string, msg sdk.Msg) (authctx.TxContext, int, error) { - txBytes, err := utils.BuildAndSignTxWithZeroGas(txCtx, name, password, []sdk.Msg{msg}) - if err != nil { - return txCtx, http.StatusInternalServerError, err - } - _, adjusted, err := utils.CalculateGas(cliCtx.Query, cliCtx.Codec, txBytes, cliCtx.GasAdjustment) - if err != nil { - return txCtx, http.StatusUnauthorized, err - } - return txCtx.WithGas(adjusted), http.StatusOK, nil -} diff --git a/x/stake/client/rest/tx.go b/x/stake/client/rest/tx.go index 729f57895..3d7d419a3 100644 --- a/x/stake/client/rest/tx.go +++ b/x/stake/client/rest/tx.go @@ -276,9 +276,9 @@ func delegationsRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx contex m.Sequence++ if m.Gas == 0 { - newCtx, httperr, err := enrichContextWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, msg) + newCtx, err := utils.EnrichCtxWithGas(txCtx, cliCtx, m.LocalAccountName, m.Password, []sdk.Msg{msg}) if err != nil { - utils.WriteErrorResponse(&w, httperr, err.Error()) + utils.WriteErrorResponse(&w, http.StatusInternalServerError, err.Error()) return } txCtx = newCtx @@ -316,15 +316,3 @@ func delegationsRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, cliCtx contex w.Write(output) } } - -func enrichContextWithGas(txCtx authcliCtx.TxContext, cliCtx context.CLIContext, name, password string, msg sdk.Msg) (authcliCtx.TxContext, int, error) { - txBytes, err := utils.BuildAndSignTxWithZeroGas(txCtx, name, password, []sdk.Msg{msg}) - if err != nil { - return txCtx, http.StatusInternalServerError, err - } - _, adjusted, err := utils.CalculateGas(cliCtx.Query, cliCtx.Codec, txBytes, cliCtx.GasAdjustment) - if err != nil { - return txCtx, http.StatusUnauthorized, err - } - return txCtx.WithGas(adjusted), http.StatusOK, nil -} From f6cb4d4fb66970c3b8e31b009db9a02e14cabae4 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Fri, 24 Aug 2018 10:18:17 -0700 Subject: [PATCH 12/25] Speedup IAVL iterator by removing defers when unneeded. Note each defer occurs a 30ish ns overhead here, which is significant for IAVL iteration. We were previously using around 3-4 defers. (2 in next, one in Valid, one in Key, one in Value) This slows down the entire application quite significantly, as we require fast iteration. --- Gopkg.lock | 53 +++++++++++++++++++++-------------------- PENDING.md | 1 + store/iavlstore.go | 35 +++++++++++++++++---------- store/iavlstore_test.go | 23 ++++++++++++++++++ 4 files changed, 73 insertions(+), 39 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 81591a0ae..fa6fafb99 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -34,11 +34,11 @@ [[projects]] branch = "master" - digest = "1:2c00f064ba355903866cbfbf3f7f4c0fe64af6638cc7d1b8bdcf3181bc67f1d8" + digest = "1:6aabc1566d6351115d561d038da82a4c19b46c3b6e17f4a0a2fa60260663dc79" name = "github.com/btcsuite/btcd" packages = ["btcec"] pruneopts = "UT" - revision = "f899737d7f2764dc13e4d01ff00108ec58f766a9" + revision = "d81d8877b8f327112e94e814937143a71d1692a7" [[projects]] digest = "1:386de157f7d19259a7f9c81f26ce011223ce0f090353c1152ffdf730d7d10ac2" @@ -71,7 +71,7 @@ version = "v1.4.7" [[projects]] - digest = "1:fdf5169073fb0ad6dc12a70c249145e30f4058647bea25f0abd48b6d9f228a11" + digest = "1:fa30c0652956e159cdb97dcb2ef8b8db63ed668c02a5c3a40961c8f0641252fe" name = "github.com/go-kit/kit" packages = [ "log", @@ -103,7 +103,7 @@ version = "v1.7.0" [[projects]] - digest = "1:35621fe20f140f05a0c4ef662c26c0ab4ee50bca78aa30fe87d33120bd28165e" + digest = "1:212285efb97b9ec2e20550d81f0446cb7897e57cbdfd7301b1363ab113d8be45" name = "github.com/gogo/protobuf" packages = [ "gogoproto", @@ -118,7 +118,7 @@ version = "v1.1.1" [[projects]] - digest = "1:17fe264ee908afc795734e8c4e63db2accabaf57326dbf21763a7d6b86096260" + digest = "1:cb22af0ed7c72d495d8be1106233ee553898950f15fd3f5404406d44c2e86888" name = "github.com/golang/protobuf" packages = [ "proto", @@ -165,12 +165,13 @@ [[projects]] branch = "master" - digest = "1:12247a2e99a060cc692f6680e5272c8adf0b8f572e6bce0d7095e624c958a240" + digest = "1:ac64f01acc5eeea9dde40e326de6b6471e501392ec06524c3b51033aa50789bc" name = "github.com/hashicorp/hcl" packages = [ ".", "hcl/ast", "hcl/parser", + "hcl/printer", "hcl/scanner", "hcl/strconv", "hcl/token", @@ -230,12 +231,12 @@ version = "v1.0.1" [[projects]] - branch = "master" - digest = "1:5ab79470a1d0fb19b041a624415612f8236b3c06070161a910562f2b2d064355" + digest = "1:645110e089152bd0f4a011a2648fbb0e4df5977be73ca605781157ac297f50c4" name = "github.com/mitchellh/mapstructure" packages = ["."] pruneopts = "UT" - revision = "f15292f7a699fcc1a38a80977f80a046874ba8ac" + revision = "fa473d140ef3c6adf42d6b391fe76707f1f243c8" + version = "v1.0.0" [[projects]] digest = "1:95741de3af260a92cc5c7f3f3061e85273f5a81b5db20d4bd68da74bd521675e" @@ -262,7 +263,7 @@ version = "v1.0.0" [[projects]] - digest = "1:c1a04665f9613e082e1209cf288bf64f4068dcd6c87a64bf1c4ff006ad422ba0" + digest = "1:98225904b7abff96c052b669b25788f18225a36673fba022fb93514bb9a2a64e" name = "github.com/prometheus/client_golang" packages = [ "prometheus", @@ -273,7 +274,7 @@ [[projects]] branch = "master" - digest = "1:2d5cd61daa5565187e1d96bae64dbbc6080dacf741448e9629c64fd93203b0d4" + digest = "1:0f37e09b3e92aaeda5991581311f8dbf38944b36a3edec61cc2d1991f527554a" name = "github.com/prometheus/client_model" packages = ["go"] pruneopts = "UT" @@ -281,7 +282,7 @@ [[projects]] branch = "master" - digest = "1:63b68062b8968092eb86bedc4e68894bd096ea6b24920faca8b9dcf451f54bb5" + digest = "1:dad2e5a2153ee7a6c9ab8fc13673a16ee4fb64434a7da980965a3741b0c981a3" name = "github.com/prometheus/common" packages = [ "expfmt", @@ -293,7 +294,7 @@ [[projects]] branch = "master" - digest = "1:8c49953a1414305f2ff5465147ee576dd705487c35b15918fcd4efdc0cb7a290" + digest = "1:a37c98f4b7a66bb5c539c0539f0915a74ef1c8e0b3b6f45735289d94cae92bfd" name = "github.com/prometheus/procfs" packages = [ ".", @@ -312,7 +313,7 @@ revision = "e2704e165165ec55d062f5919b4b29494e9fa790" [[projects]] - digest = "1:bd1ae00087d17c5a748660b8e89e1043e1e5479d0fea743352cda2f8dd8c4f84" + digest = "1:37ace7f35375adec11634126944bdc45a673415e2fcc07382d03b75ec76ea94c" name = "github.com/spf13/afero" packages = [ ".", @@ -331,7 +332,7 @@ version = "v1.2.0" [[projects]] - digest = "1:7ffc0983035bc7e297da3688d9fe19d60a420e9c38bef23f845c53788ed6a05e" + digest = "1:627ab2f549a6a55c44f46fa24a4307f4d0da81bfc7934ed0473bf38b24051d26" name = "github.com/spf13/cobra" packages = ["."] pruneopts = "UT" @@ -363,7 +364,7 @@ version = "v1.0.0" [[projects]] - digest = "1:7e8d267900c7fa7f35129a2a37596e38ed0f11ca746d6d9ba727980ee138f9f6" + digest = "1:73697231b93fb74a73ebd8384b68b9a60c57ea6b13c56d2425414566a72c8e6d" name = "github.com/stretchr/testify" packages = [ "assert", @@ -375,7 +376,7 @@ [[projects]] branch = "master" - digest = "1:f2ffd421680b0a3f7887501b3c6974bcf19217ecd301d0e2c9b681940ec363d5" + digest = "1:442d2ffa75ffae302ce8800bf4144696b92bef02917923ea132ce2d39efe7d65" name = "github.com/syndtr/goleveldb" packages = [ "leveldb", @@ -396,7 +397,7 @@ [[projects]] branch = "master" - digest = "1:087aaa7920e5d0bf79586feb57ce01c35c830396ab4392798112e8aae8c47722" + digest = "1:203b409c21115233a576f99e8f13d8e07ad82b25500491f7e1cca12588fb3232" name = "github.com/tendermint/ed25519" packages = [ ".", @@ -423,7 +424,7 @@ version = "v0.9.2" [[projects]] - digest = "1:4f15e95fe3888cc75dd34f407d6394cbc7fd3ff24920851b92b295f6a8b556e6" + digest = "1:963f6c04345ce36f900c1d6367200eebc3cc2db6ee632ff865ea8dcf64b748a0" name = "github.com/tendermint/tendermint" packages = [ "abci/client", @@ -490,7 +491,7 @@ version = "v0.23.1-rc0" [[projects]] - digest = "1:bf6d9a827ea3cad964c2f863302e4f6823170d0b5ed16f72cf1184a7c615067e" + digest = "1:ad879bb8c71020a3f92f0c61f414d93eae1d5dc2f37023b6abaa3cc84b00165e" name = "github.com/tendermint/tmlibs" packages = ["cli"] pruneopts = "UT" @@ -506,7 +507,7 @@ [[projects]] branch = "master" - digest = "1:27507554c6d4f060d8d700c31c624a43d3a92baa634e178ddc044bdf7d13b44a" + digest = "1:2a3ce1f08dcae8bac666deb6e4c88b5d7170c510da38fd746231144cac351704" name = "golang.org/x/crypto" packages = [ "blowfish", @@ -528,7 +529,7 @@ revision = "614d502a4dac94afa3a6ce146bd1736da82514c6" [[projects]] - digest = "1:d36f55a999540d29b6ea3c2ea29d71c76b1d9853fdcd3e5c5cb4836f2ba118f1" + digest = "1:04dda8391c3e2397daf254ac68003f30141c069b228d06baec8324a5f81dc1e9" name = "golang.org/x/net" packages = [ "context", @@ -545,17 +546,17 @@ [[projects]] branch = "master" - digest = "1:a0e12bc26f317c0e2d497baf767285e1790e526e8dd46553c5a67fcbc8692157" + digest = "1:c8baf78f0ac6eb27c645e264fe5e8a74d5a50db188ab41a7ff3b275c112e0735" name = "golang.org/x/sys" packages = [ "cpu", "unix", ] pruneopts = "UT" - revision = "3b58ed4ad3395d483fc92d5d14123ce2c3581fec" + revision = "11551d06cbcc94edc80a0facaccbda56473c19c1" [[projects]] - digest = "1:a2ab62866c75542dd18d2b069fec854577a20211d7c0ea6ae746072a1dccdd18" + digest = "1:7509ba4347d1f8de6ae9be8818b0cd1abc3deeffe28aeaf4be6d4b6b5178d9ca" name = "golang.org/x/text" packages = [ "collate", @@ -586,7 +587,7 @@ revision = "c66870c02cf823ceb633bcd05be3c7cda29976f4" [[projects]] - digest = "1:2dab32a43451e320e49608ff4542fdfc653c95dcc35d0065ec9c6c3dd540ed74" + digest = "1:4515e3030c440845b046354fd5d57671238428b820deebce2e9dabb5cd3c51ac" name = "google.golang.org/grpc" packages = [ ".", diff --git a/PENDING.md b/PENDING.md index 01bdc2158..426f368a3 100644 --- a/PENDING.md +++ b/PENDING.md @@ -59,6 +59,7 @@ IMPROVEMENTS * SDK * [tools] Make get_vendor_deps deletes `.vendor-new` directories, in case scratch files are present. * [cli] \#1632 Add integration tests to ensure `basecoind init && basecoind` start sequences run successfully for both `democoin` and `basecoin` examples. + * [store] Speedup IAVL iteration, and consequently everything that requires IAVL iteration. [#2143](https://github.com/cosmos/cosmos-sdk/issues/2143) * Tendermint diff --git a/store/iavlstore.go b/store/iavlstore.go index 6ab50dfc9..daffa7dd5 100644 --- a/store/iavlstore.go +++ b/store/iavlstore.go @@ -332,39 +332,42 @@ func (iter *iavlIterator) Domain() (start, end []byte) { func (iter *iavlIterator) Valid() bool { iter.waitInit() iter.mtx.Lock() - defer iter.mtx.Unlock() - return !iter.invalid + validity := !iter.invalid + iter.mtx.Unlock() + return validity } // Implements Iterator. func (iter *iavlIterator) Next() { iter.waitInit() iter.mtx.Lock() - defer iter.mtx.Unlock() - iter.assertIsValid() + iter.assertIsValid(true) iter.receiveNext() + iter.mtx.Unlock() } // Implements Iterator. func (iter *iavlIterator) Key() []byte { iter.waitInit() iter.mtx.Lock() - defer iter.mtx.Unlock() - iter.assertIsValid() + iter.assertIsValid(true) - return iter.key + key := iter.key + iter.mtx.Unlock() + return key } // Implements Iterator. func (iter *iavlIterator) Value() []byte { iter.waitInit() iter.mtx.Lock() - defer iter.mtx.Unlock() - iter.assertIsValid() + iter.assertIsValid(true) - return iter.value + val := iter.value + iter.mtx.Unlock() + return val } // Implements Iterator. @@ -375,14 +378,14 @@ func (iter *iavlIterator) Close() { //---------------------------------------- func (iter *iavlIterator) setNext(key, value []byte) { - iter.assertIsValid() + iter.assertIsValid(false) iter.key = key iter.value = value } func (iter *iavlIterator) setInvalid() { - iter.assertIsValid() + iter.assertIsValid(false) iter.invalid = true } @@ -400,8 +403,14 @@ func (iter *iavlIterator) receiveNext() { } } -func (iter *iavlIterator) assertIsValid() { +// assertIsValid panics if the iterator is invalid. If unlockMutex is true, +// it also unlocks the mutex before panicing, to prevent deadlocks in code that +// recovers from panics +func (iter *iavlIterator) assertIsValid(unlockMutex bool) { if iter.invalid { + if unlockMutex { + iter.mtx.Unlock() + } panic("invalid iterator") } } diff --git a/store/iavlstore_test.go b/store/iavlstore_test.go index 38d85c658..793089a26 100644 --- a/store/iavlstore_test.go +++ b/store/iavlstore_test.go @@ -464,3 +464,26 @@ func TestIAVLStoreQuery(t *testing.T) { require.Equal(t, uint32(sdk.CodeOK), qres.Code) require.Equal(t, v1, qres.Value) } + +func BenchmarkIAVLIteratorNext(b *testing.B) { + db := dbm.NewMemDB() + treeSize := 1000 + tree := iavl.NewVersionedTree(db, cacheSize) + for i := 0; i < treeSize; i++ { + key := cmn.RandBytes(4) + value := cmn.RandBytes(50) + tree.Set(key, value) + } + iavlStore := newIAVLStore(tree, numRecent, storeEvery) + iterators := make([]Iterator, b.N/treeSize) + for i := 0; i < len(iterators); i++ { + iterators[i] = iavlStore.Iterator([]byte{0}, []byte{255, 255, 255, 255, 255}) + } + b.ResetTimer() + for i := 0; i < len(iterators); i++ { + iter := iterators[i] + for j := 0; j < treeSize; j++ { + iter.Next() + } + } +} From 639161100515156455c5349096268d4f61f8caa7 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Wed, 22 Aug 2018 18:44:35 -0700 Subject: [PATCH 13/25] asdf --- x/gov/client/cli/tx.go | 246 +++++++++++++++++++++++--------------- x/gov/client/rest/rest.go | 43 +++++++ 2 files changed, 190 insertions(+), 99 deletions(-) diff --git a/x/gov/client/cli/tx.go b/x/gov/client/cli/tx.go index 2a7e71aac..26883b9c3 100644 --- a/x/gov/client/cli/tx.go +++ b/x/gov/client/cli/tx.go @@ -13,11 +13,11 @@ import ( "github.com/cosmos/cosmos-sdk/x/gov" "encoding/json" - "github.com/pkg/errors" - "github.com/spf13/cobra" - "github.com/spf13/viper" "io/ioutil" "strings" + + "github.com/spf13/cobra" + "github.com/spf13/viper" ) const ( @@ -252,20 +252,21 @@ func GetCmdQueryProposal(storeName string, cdc *wire.Codec) *cobra.Command { cliCtx := context.NewCLIContext().WithCodec(cdc) proposalID := viper.GetInt64(flagProposalID) - res, err := cliCtx.QueryStore(gov.KeyProposal(proposalID), storeName) - if len(res) == 0 || err != nil { - return errors.Errorf("proposalID [%d] is not existed", proposalID) + params := gov.QueryProposalParams{ + ProposalID: proposalID, } - var proposal gov.Proposal - cdc.MustUnmarshalBinary(res, &proposal) - - output, err := wire.MarshalJSONIndent(cdc, proposal) + bz, err := cdc.MarshalJSON(params) if err != nil { return err } - fmt.Println(string(output)) + res, err := cliCtx.QueryWithData("custom/gov/proposal", bz) + if err != nil { + return err + } + + fmt.Println(string(res)) return nil }, } @@ -287,88 +288,47 @@ func GetCmdQueryProposals(storeName string, cdc *wire.Codec) *cobra.Command { strProposalStatus := viper.GetString(flagStatus) latestProposalsIDs := viper.GetInt64(flagLatestProposalIDs) - var err error - var voterAddr sdk.AccAddress - var depositerAddr sdk.AccAddress - var proposalStatus gov.ProposalStatus + params := gov.QueryProposalsParams{ + NumLatestProposals: latestProposalsIDs, + } if len(bechDepositerAddr) != 0 { - depositerAddr, err = sdk.AccAddressFromBech32(bechDepositerAddr) + depositerAddr, err := sdk.AccAddressFromBech32(bechDepositerAddr) if err != nil { return err } + params.Depositer = depositerAddr } if len(bechVoterAddr) != 0 { - voterAddr, err = sdk.AccAddressFromBech32(bechVoterAddr) + voterAddr, err := sdk.AccAddressFromBech32(bechVoterAddr) if err != nil { return err } + params.Voter = voterAddr } if len(strProposalStatus) != 0 { - proposalStatus, err = gov.ProposalStatusFromString(strProposalStatus) + proposalStatus, err := gov.ProposalStatusFromString(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.QueryStore(gov.KeyNextProposalID, storeName) + res, err := cliCtx.QueryWithData("custom/gov/proposals", bz) if err != nil { return err } - var maxProposalID int64 - cdc.MustUnmarshalBinary(res, &maxProposalID) - - matchingProposals := []gov.Proposal{} - - if latestProposalsIDs == 0 { - latestProposalsIDs = maxProposalID - } - - for proposalID := maxProposalID - latestProposalsIDs; proposalID < maxProposalID; proposalID++ { - if voterAddr != nil { - res, err = cliCtx.QueryStore(gov.KeyVote(proposalID, voterAddr), storeName) - if err != nil || len(res) == 0 { - continue - } - } - - if depositerAddr != nil { - res, err = cliCtx.QueryStore(gov.KeyDeposit(proposalID, depositerAddr), storeName) - if err != nil || len(res) == 0 { - continue - } - } - - res, err = cliCtx.QueryStore(gov.KeyProposal(proposalID), storeName) - if err != nil || len(res) == 0 { - continue - } - - var proposal gov.Proposal - cdc.MustUnmarshalBinary(res, &proposal) - - if len(strProposalStatus) != 0 { - if proposal.GetStatus() != proposalStatus { - continue - } - } - - matchingProposals = append(matchingProposals, proposal) - } - - 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()) - } + fmt.Println(string(res)) return nil }, } @@ -396,20 +356,21 @@ func GetCmdQueryVote(storeName string, cdc *wire.Codec) *cobra.Command { return err } - res, err := cliCtx.QueryStore(gov.KeyVote(proposalID, voterAddr), storeName) - if len(res) == 0 || err != nil { - return errors.Errorf("proposalID [%d] does not exist", proposalID) + params := gov.QueryVoteParams{ + Voter: voterAddr, + ProposalID: proposalID, } - - var vote gov.Vote - cdc.MustUnmarshalBinary(res, &vote) - - output, err := wire.MarshalJSONIndent(cdc, vote) + bz, err := cdc.MarshalJSON(params) if err != nil { return err } - fmt.Println(string(output)) + res, err := cliCtx.QueryWithData("custom/gov/vote", bz) + if err != nil { + return err + } + + fmt.Println(string(res)) return nil }, } @@ -429,37 +390,20 @@ func GetCmdQueryVotes(storeName string, cdc *wire.Codec) *cobra.Command { cliCtx := context.NewCLIContext().WithCodec(cdc) proposalID := viper.GetInt64(flagProposalID) - res, err := cliCtx.QueryStore(gov.KeyProposal(proposalID), storeName) - if len(res) == 0 || err != nil { - return errors.Errorf("proposalID [%d] does not exist", proposalID) + params := gov.QueryVotesParams{ + ProposalID: proposalID, } - - var proposal gov.Proposal - cdc.MustUnmarshalBinary(res, &proposal) - - if proposal.GetStatus() != gov.StatusVotingPeriod { - fmt.Println("Proposal not in voting period.") - return nil - } - - res2, err := cliCtx.QuerySubspace(gov.KeyVotesSubspace(proposalID), storeName) + bz, err := cdc.MarshalJSON(params) if err != nil { return err } - var votes []gov.Vote - for i := 0; i < len(res2); i++ { - var vote gov.Vote - cdc.MustUnmarshalBinary(res2[i].Value, &vote) - votes = append(votes, vote) - } - - output, err := wire.MarshalJSONIndent(cdc, votes) + res, err := cliCtx.QueryWithData("custom/gov/votes", bz) if err != nil { return err } - fmt.Println(string(output)) + fmt.Println(string(res)) return nil }, } @@ -468,3 +412,107 @@ func GetCmdQueryVotes(storeName string, cdc *wire.Codec) *cobra.Command { return cmd } + +// Command to Get a specific Deposit Information +// GetCmdQueryDeposit implements the query proposal deposit command. +func GetCmdQueryDeposit(storeName string, cdc *wire.Codec) *cobra.Command { + cmd := &cobra.Command{ + Use: "query-deposit", + Short: "query deposit", + RunE: func(cmd *cobra.Command, args []string) error { + cliCtx := context.NewCLIContext().WithCodec(cdc) + proposalID := 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("custom/gov/deposit", 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(storeName string, cdc *wire.Codec) *cobra.Command { + cmd := &cobra.Command{ + Use: "query-deposits", + Short: "query deposits on a proposal", + RunE: func(cmd *cobra.Command, args []string) error { + cliCtx := context.NewCLIContext().WithCodec(cdc) + proposalID := viper.GetInt64(flagProposalID) + + params := gov.QueryDepositsParams{ + ProposalID: proposalID, + } + bz, err := cdc.MarshalJSON(params) + if err != nil { + return err + } + + res, err := cliCtx.QueryWithData("custom/gov/deposits", 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 +} + +// GetCmdQueryDeposits implements the command to query for proposal deposits. +func GetCmdQueryTally(storeName string, cdc *wire.Codec) *cobra.Command { + cmd := &cobra.Command{ + Use: "query-tally", + Short: "get the tally of a proposal vote", + RunE: func(cmd *cobra.Command, args []string) error { + cliCtx := context.NewCLIContext().WithCodec(cdc) + proposalID := viper.GetInt64(flagProposalID) + + params := gov.QueryTallyParams{ + ProposalID: proposalID, + } + bz, err := cdc.MarshalJSON(params) + if err != nil { + return err + } + + res, err := cliCtx.QueryWithData("custom/gov/tally", 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/rest/rest.go b/x/gov/client/rest/rest.go index 5cdc7bda2..ebe612314 100644 --- a/x/gov/client/rest/rest.go +++ b/x/gov/client/rest/rest.go @@ -479,3 +479,46 @@ func queryProposalsWithParameterFn(cdc *wire.Codec) http.HandlerFunc { w.Write(res) } } + +// nolint: gocyclo +// todo: Split this functionality into helper functions to remove the above +func queryTallyOnProposalHandlerFn(cdc *wire.Codec) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + vars := mux.Vars(r) + strProposalID := vars[RestProposalID] + + if len(strProposalID) == 0 { + w.WriteHeader(http.StatusBadRequest) + err := errors.New("proposalId required but not specified") + w.Write([]byte(err.Error())) + + return + } + + proposalID, ok := parseInt64OrReturnBadRequest(strProposalID, w) + if !ok { + return + } + + cliCtx := context.NewCLIContext().WithCodec(cdc) + + params := gov.QueryTallyParams{ + ProposalID: proposalID, + } + bz, err := cdc.MarshalJSON(params) + if err != nil { + w.WriteHeader(http.StatusBadRequest) + w.Write([]byte(err.Error())) + return + } + + res, err := cliCtx.QueryWithData("custom/gov/tally", bz) + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte(err.Error())) + return + } + + w.Write(res) + } +} From 76a16ab288b388eb8a036313f2efcfff410819db Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Sat, 25 Aug 2018 20:12:14 +0100 Subject: [PATCH 14/25] Modify AnteHandler to take a simulate boolean parameter --- baseapp/baseapp.go | 2 +- baseapp/baseapp_test.go | 12 ++--- client/flags.go | 3 +- client/utils/utils_test.go | 38 ++++++++++++++++ docs/sdk/core/examples/app2.go | 2 +- types/handler.go | 2 +- x/auth/ante.go | 4 +- x/auth/ante_test.go | 80 +++++++++++++++++----------------- x/params/msg_status.go | 2 +- 9 files changed, 93 insertions(+), 52 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 20faf06cd..60c694ff7 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -608,7 +608,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk // run the ante handler if app.anteHandler != nil { - newCtx, result, abort := app.anteHandler(ctx, tx) + newCtx, result, abort := app.anteHandler(ctx, tx, (mode == runTxModeSimulate)) if abort { return result } diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 269424e58..04e47214d 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -365,7 +365,7 @@ func testTxDecoder(cdc *wire.Codec) sdk.TxDecoder { } func anteHandlerTxTest(t *testing.T, capKey *sdk.KVStoreKey, storeKey []byte) sdk.AnteHandler { - return func(ctx sdk.Context, tx sdk.Tx) (newCtx sdk.Context, res sdk.Result, abort bool) { + 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) @@ -595,7 +595,7 @@ func TestSimulateTx(t *testing.T) { gasConsumed := int64(5) anteOpt := func(bapp *BaseApp) { - bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx) (newCtx sdk.Context, res sdk.Result, abort bool) { + bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) { newCtx = ctx.WithGasMeter(sdk.NewGasMeter(gasConsumed)) return }) @@ -659,7 +659,9 @@ func TestSimulateTx(t *testing.T) { func TestRunInvalidTransaction(t *testing.T) { anteOpt := func(bapp *BaseApp) { - bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx) (newCtx sdk.Context, res sdk.Result, abort bool) { return }) + bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) { + return + }) } routerOpt := func(bapp *BaseApp) { bapp.Router().AddRoute(typeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (res sdk.Result) { return }) @@ -734,7 +736,7 @@ func TestRunInvalidTransaction(t *testing.T) { func TestTxGasLimits(t *testing.T) { gasGranted := int64(10) anteOpt := func(bapp *BaseApp) { - bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx) (newCtx sdk.Context, res sdk.Result, abort bool) { + 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)) // NOTE/TODO/XXX: @@ -825,7 +827,7 @@ func TestTxGasLimits(t *testing.T) { func TestQuery(t *testing.T) { key, value := []byte("hello"), []byte("goodbye") anteOpt := func(bapp *BaseApp) { - bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx) (newCtx sdk.Context, res sdk.Result, abort bool) { + bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) { store := ctx.KVStore(capKey1) store.Set(key, value) return diff --git a/client/flags.go b/client/flags.go index 7337e9741..81e067067 100644 --- a/client/flags.go +++ b/client/flags.go @@ -4,6 +4,7 @@ import "github.com/spf13/cobra" // nolint const ( + DefaultGasLimit = 200000 DefaultGasAdjustment = 1.2 FlagUseLedger = "ledger" @@ -52,7 +53,7 @@ func PostCommands(cmds ...*cobra.Command) []*cobra.Command { c.Flags().String(FlagChainID, "", "Chain ID of tendermint node") c.Flags().String(FlagNode, "tcp://localhost:26657", ": to tendermint rpc interface for this chain") c.Flags().Bool(FlagUseLedger, false, "Use a connected Ledger device") - c.Flags().Int64(FlagGas, 0, "gas limit to set per-transaction; set to 0 to calculate required gas automatically") + c.Flags().Int64(FlagGas, DefaultGasLimit, "gas limit to set per-transaction; set to 0 to calculate required gas automatically") c.Flags().Float64(FlagGasAdjustment, DefaultGasAdjustment, "adjustment factor to be multiplied against the estimate returned by the tx simulation") c.Flags().Bool(FlagAsync, false, "broadcast transactions asynchronously") c.Flags().Bool(FlagJson, false, "return output in json format") diff --git a/client/utils/utils_test.go b/client/utils/utils_test.go index d9ea44488..731ded903 100644 --- a/client/utils/utils_test.go +++ b/client/utils/utils_test.go @@ -1,11 +1,13 @@ package utils import ( + "errors" "testing" "github.com/cosmos/cosmos-sdk/cmd/gaia/app" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/assert" + "github.com/tendermint/tendermint/libs/common" ) func TestParseQueryResponse(t *testing.T) { @@ -18,3 +20,39 @@ func TestParseQueryResponse(t *testing.T) { assert.Equal(t, gas, int64(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) { + return func(string, common.HexBytes) ([]byte, error) { + if wantErr { + return nil, errors.New("") + } + return cdc.MustMarshalBinary(sdk.Result{GasUsed: gasUsed}), nil + } + } + type args struct { + queryFuncGasUsed int64 + queryFuncWantErr bool + adjustment float64 + } + tests := []struct { + name string + args args + wantEstimate int64 + wantAdjusted int64 + wantErr bool + }{ + {"error", args{0, true, 1.2}, 0, 0, true}, + {"adjusted gas", args{10, false, 1.2}, 10, 12, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + queryFunc := makeQueryFunc(tt.args.queryFuncGasUsed, tt.args.queryFuncWantErr) + gotEstimate, gotAdjusted, err := CalculateGas(queryFunc, cdc, []byte(""), tt.args.adjustment) + assert.Equal(t, err != nil, tt.wantErr) + assert.Equal(t, gotEstimate, tt.wantEstimate) + assert.Equal(t, gotAdjusted, tt.wantAdjusted) + }) + } +} diff --git a/docs/sdk/core/examples/app2.go b/docs/sdk/core/examples/app2.go index 3c7f71f6d..5f23abe07 100644 --- a/docs/sdk/core/examples/app2.go +++ b/docs/sdk/core/examples/app2.go @@ -211,7 +211,7 @@ func tx2Decoder(cdc *wire.Codec) sdk.TxDecoder { // Simple anteHandler that ensures msg signers have signed. // Provides no replay protection. -func antehandler(ctx sdk.Context, tx sdk.Tx) (_ sdk.Context, _ sdk.Result, abort bool) { +func antehandler(ctx sdk.Context, tx sdk.Tx, simulate bool) (_ sdk.Context, _ sdk.Result, abort bool) { appTx, ok := tx.(app2Tx) if !ok { // set abort boolean to true so that we don't continue to process failed tx diff --git a/types/handler.go b/types/handler.go index 3a50e0ce0..b978e8e51 100644 --- a/types/handler.go +++ b/types/handler.go @@ -5,4 +5,4 @@ type Handler func(ctx Context, msg Msg) Result // AnteHandler authenticates transactions, before their internal messages are handled. // If newCtx.IsZero(), ctx is used instead. -type AnteHandler func(ctx Context, tx Tx) (newCtx Context, result Result, abort bool) +type AnteHandler func(ctx Context, tx Tx, simulate bool) (newCtx Context, result Result, abort bool) diff --git a/x/auth/ante.go b/x/auth/ante.go index dd291a4a6..c0a129be3 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -25,7 +25,7 @@ const ( func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { return func( - ctx sdk.Context, tx sdk.Tx, + ctx sdk.Context, tx sdk.Tx, simulate bool, ) (newCtx sdk.Context, res sdk.Result, abort bool) { // This AnteHandler requires Txs to be StdTxs @@ -35,7 +35,7 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { } // set the gas meter - if stdTx.Fee.Gas == 0 { + if simulate { newCtx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) } else { newCtx = ctx.WithGasMeter(sdk.NewGasMeter(stdTx.Fee.Gas)) diff --git a/x/auth/ante_test.go b/x/auth/ante_test.go index 25d31b067..a841bc776 100644 --- a/x/auth/ante_test.go +++ b/x/auth/ante_test.go @@ -39,16 +39,16 @@ func privAndAddr() (crypto.PrivKey, sdk.AccAddress) { } // run the tx through the anteHandler and ensure its valid -func checkValidTx(t *testing.T, anteHandler sdk.AnteHandler, ctx sdk.Context, tx sdk.Tx) { - _, result, abort := anteHandler(ctx, tx) +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.True(t, result.IsOK()) } // run the tx through the anteHandler and ensure it fails with the given code -func checkInvalidTx(t *testing.T, anteHandler sdk.AnteHandler, ctx sdk.Context, tx sdk.Tx, code sdk.CodeType) { - newCtx, result, abort := anteHandler(ctx, 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)) @@ -140,23 +140,23 @@ func TestAnteHandlerSigErrors(t *testing.T) { require.Equal(t, expectedSigners, stdTx.GetSigners()) // Check no signatures fails - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeUnauthorized) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeUnauthorized) // test num sigs dont match GetSigners privs, accNums, seqs = []crypto.PrivKey{priv1}, []int64{0}, []int64{0} tx = newTestTx(ctx, msgs, privs, accNums, seqs, fee) - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeUnauthorized) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeUnauthorized) // test an unrecognized account privs, accNums, seqs = []crypto.PrivKey{priv1, priv2, priv3}, []int64{0, 1, 2}, []int64{0, 0, 0} tx = newTestTx(ctx, msgs, privs, accNums, seqs, fee) - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeUnknownAddress) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeUnknownAddress) // save the first account, but second is still unrecognized acc1 := mapper.NewAccountWithAddress(ctx, addr1) acc1.SetCoins(fee.Amount) mapper.SetAccount(ctx, acc1) - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeUnknownAddress) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeUnknownAddress) } // Test logic around account number checking with one signer and many signers. @@ -192,17 +192,17 @@ func TestAnteHandlerAccountNumbers(t *testing.T) { // test good tx from one signer privs, accnums, seqs := []crypto.PrivKey{priv1}, []int64{0}, []int64{0} tx = newTestTx(ctx, msgs, privs, accnums, seqs, fee) - checkValidTx(t, anteHandler, ctx, tx) + checkValidTx(t, anteHandler, ctx, tx, false) // new tx from wrong account number seqs = []int64{1} tx = newTestTx(ctx, msgs, privs, []int64{1}, seqs, fee) - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInvalidSequence) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeInvalidSequence) // from correct account number seqs = []int64{1} tx = newTestTx(ctx, msgs, privs, []int64{0}, seqs, fee) - checkValidTx(t, anteHandler, ctx, tx) + checkValidTx(t, anteHandler, ctx, tx, false) // new tx with another signer and incorrect account numbers msg1 := newTestMsg(addr1, addr2) @@ -210,12 +210,12 @@ func TestAnteHandlerAccountNumbers(t *testing.T) { msgs = []sdk.Msg{msg1, msg2} privs, accnums, seqs = []crypto.PrivKey{priv1, priv2}, []int64{1, 0}, []int64{2, 0} tx = newTestTx(ctx, msgs, privs, accnums, seqs, fee) - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInvalidSequence) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeInvalidSequence) // correct account numbers privs, accnums, seqs = []crypto.PrivKey{priv1, priv2}, []int64{0, 1}, []int64{2, 0} tx = newTestTx(ctx, msgs, privs, accnums, seqs, fee) - checkValidTx(t, anteHandler, ctx, tx) + checkValidTx(t, anteHandler, ctx, tx, false) } // Test logic around sequence checking with one signer and many signers. @@ -255,15 +255,15 @@ func TestAnteHandlerSequences(t *testing.T) { // test good tx from one signer privs, accnums, seqs := []crypto.PrivKey{priv1}, []int64{0}, []int64{0} tx = newTestTx(ctx, msgs, privs, accnums, seqs, fee) - checkValidTx(t, anteHandler, ctx, tx) + checkValidTx(t, anteHandler, ctx, tx, false) // test sending it again fails (replay protection) - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInvalidSequence) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeInvalidSequence) // fix sequence, should pass seqs = []int64{1} tx = newTestTx(ctx, msgs, privs, accnums, seqs, fee) - checkValidTx(t, anteHandler, ctx, tx) + checkValidTx(t, anteHandler, ctx, tx, false) // new tx with another signer and correct sequences msg1 := newTestMsg(addr1, addr2) @@ -272,28 +272,28 @@ func TestAnteHandlerSequences(t *testing.T) { privs, accnums, seqs = []crypto.PrivKey{priv1, priv2, priv3}, []int64{0, 1, 2}, []int64{2, 0, 0} tx = newTestTx(ctx, msgs, privs, accnums, seqs, fee) - checkValidTx(t, anteHandler, ctx, tx) + checkValidTx(t, anteHandler, ctx, tx, false) // replay fails - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInvalidSequence) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeInvalidSequence) // tx from just second signer with incorrect sequence fails msg = newTestMsg(addr2) msgs = []sdk.Msg{msg} privs, accnums, seqs = []crypto.PrivKey{priv2}, []int64{1}, []int64{0} tx = newTestTx(ctx, msgs, privs, accnums, seqs, fee) - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInvalidSequence) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeInvalidSequence) // fix the sequence and it passes tx = newTestTx(ctx, msgs, []crypto.PrivKey{priv2}, []int64{1}, []int64{1}, fee) - checkValidTx(t, anteHandler, ctx, tx) + checkValidTx(t, anteHandler, ctx, tx, false) // another tx from both of them that passes msg = newTestMsg(addr1, addr2) msgs = []sdk.Msg{msg} privs, accnums, seqs = []crypto.PrivKey{priv1, priv2}, []int64{0, 1}, []int64{3, 2} tx = newTestTx(ctx, msgs, privs, accnums, seqs, fee) - checkValidTx(t, anteHandler, ctx, tx) + checkValidTx(t, anteHandler, ctx, tx, false) } // Test logic around fee deduction. @@ -323,17 +323,17 @@ func TestAnteHandlerFees(t *testing.T) { // signer does not have enough funds to pay the fee tx = newTestTx(ctx, msgs, privs, accnums, seqs, fee) - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInsufficientFunds) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeInsufficientFunds) acc1.SetCoins(sdk.Coins{sdk.NewInt64Coin("atom", 149)}) mapper.SetAccount(ctx, acc1) - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInsufficientFunds) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeInsufficientFunds) require.True(t, feeCollector.GetCollectedFees(ctx).IsEqual(emptyCoins)) acc1.SetCoins(sdk.Coins{sdk.NewInt64Coin("atom", 150)}) mapper.SetAccount(ctx, acc1) - checkValidTx(t, anteHandler, ctx, tx) + checkValidTx(t, anteHandler, ctx, tx, false) require.True(t, feeCollector.GetCollectedFees(ctx).IsEqual(sdk.Coins{sdk.NewInt64Coin("atom", 150)})) } @@ -360,26 +360,26 @@ func TestAnteHandlerMemoGas(t *testing.T) { var tx sdk.Tx msg := newTestMsg(addr1) privs, accnums, seqs := []crypto.PrivKey{priv1}, []int64{0}, []int64{0} - fee := NewStdFee(1, sdk.NewInt64Coin("atom", 0)) + fee := NewStdFee(0, sdk.NewInt64Coin("atom", 0)) // tx does not have enough gas tx = newTestTx(ctx, []sdk.Msg{msg}, privs, accnums, seqs, fee) - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeOutOfGas) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeOutOfGas) // tx with memo doesn't have enough gas fee = NewStdFee(801, sdk.NewInt64Coin("atom", 0)) tx = newTestTxWithMemo(ctx, []sdk.Msg{msg}, privs, accnums, seqs, fee, "abcininasidniandsinasindiansdiansdinaisndiasndiadninsd") - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeOutOfGas) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeOutOfGas) // memo too large fee = NewStdFee(2001, sdk.NewInt64Coin("atom", 0)) tx = newTestTxWithMemo(ctx, []sdk.Msg{msg}, privs, accnums, seqs, fee, "abcininasidniandsinasindiansdiansdinaisndiasndiadninsdabcininasidniandsinasindiansdiansdinaisndiasndiadninsdabcininasidniandsinasindiansdiansdinaisndiasndiadninsd") - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeMemoTooLarge) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeMemoTooLarge) // tx with memo has enough gas fee = NewStdFee(1100, sdk.NewInt64Coin("atom", 0)) tx = newTestTxWithMemo(ctx, []sdk.Msg{msg}, privs, accnums, seqs, fee, "abcininasidniandsinasindiansdiansdinaisndiasndiadninsd") - checkValidTx(t, anteHandler, ctx, tx) + checkValidTx(t, anteHandler, ctx, tx, false) } func TestAnteHandlerMultiSigner(t *testing.T) { @@ -420,17 +420,17 @@ func TestAnteHandlerMultiSigner(t *testing.T) { privs, accnums, seqs := []crypto.PrivKey{priv1, priv2, priv3}, []int64{0, 1, 2}, []int64{0, 0, 0} tx = newTestTxWithMemo(ctx, msgs, privs, accnums, seqs, fee, "Check signers are in expected order and different account numbers works") - checkValidTx(t, anteHandler, ctx, tx) + checkValidTx(t, anteHandler, ctx, tx, false) // change sequence numbers tx = newTestTx(ctx, []sdk.Msg{msg1}, []crypto.PrivKey{priv1, priv2}, []int64{0, 1}, []int64{1, 1}, fee) - checkValidTx(t, anteHandler, ctx, tx) + checkValidTx(t, anteHandler, ctx, tx, false) tx = newTestTx(ctx, []sdk.Msg{msg2}, []crypto.PrivKey{priv3, priv1}, []int64{2, 0}, []int64{1, 2}, fee) - checkValidTx(t, anteHandler, ctx, tx) + checkValidTx(t, anteHandler, ctx, tx, false) // expected seqs = [3, 2, 2] tx = newTestTxWithMemo(ctx, msgs, privs, accnums, []int64{3, 2, 2}, fee, "Check signers are in expected order and different account numbers and sequence numbers works") - checkValidTx(t, anteHandler, ctx, tx) + checkValidTx(t, anteHandler, ctx, tx, false) } func TestAnteHandlerBadSignBytes(t *testing.T) { @@ -467,7 +467,7 @@ func TestAnteHandlerBadSignBytes(t *testing.T) { // test good tx and signBytes privs, accnums, seqs := []crypto.PrivKey{priv1}, []int64{0}, []int64{0} tx = newTestTx(ctx, msgs, privs, accnums, seqs, fee) - checkValidTx(t, anteHandler, ctx, tx) + checkValidTx(t, anteHandler, ctx, tx, false) chainID := ctx.ChainID() chainID2 := chainID + "somemorestuff" @@ -497,20 +497,20 @@ func TestAnteHandlerBadSignBytes(t *testing.T) { StdSignBytes(cs.chainID, cs.accnum, cs.seq, cs.fee, cs.msgs, ""), "", ) - checkInvalidTx(t, anteHandler, ctx, tx, cs.code) + checkInvalidTx(t, anteHandler, ctx, tx, false, cs.code) } // test wrong signer if public key exist privs, accnums, seqs = []crypto.PrivKey{priv2}, []int64{0}, []int64{1} tx = newTestTx(ctx, msgs, privs, accnums, seqs, fee) - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeUnauthorized) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeUnauthorized) // test wrong signer if public doesn't exist msg = newTestMsg(addr2) msgs = []sdk.Msg{msg} privs, accnums, seqs = []crypto.PrivKey{priv1}, []int64{1}, []int64{0} tx = newTestTx(ctx, msgs, privs, accnums, seqs, fee) - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInvalidPubKey) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeInvalidPubKey) } @@ -544,7 +544,7 @@ func TestAnteHandlerSetPubKey(t *testing.T) { privs, accnums, seqs := []crypto.PrivKey{priv1}, []int64{0}, []int64{0} fee := newStdFee() tx = newTestTx(ctx, msgs, privs, accnums, seqs, fee) - checkValidTx(t, anteHandler, ctx, tx) + checkValidTx(t, anteHandler, ctx, tx, false) acc1 = mapper.GetAccount(ctx, addr1) require.Equal(t, acc1.GetPubKey(), priv1.PubKey()) @@ -555,14 +555,14 @@ func TestAnteHandlerSetPubKey(t *testing.T) { tx = newTestTx(ctx, msgs, privs, []int64{1}, seqs, fee) sigs := tx.(StdTx).GetSignatures() sigs[0].PubKey = nil - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInvalidPubKey) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeInvalidPubKey) acc2 = mapper.GetAccount(ctx, addr2) require.Nil(t, acc2.GetPubKey()) // test invalid signature and public key tx = newTestTx(ctx, msgs, privs, []int64{1}, seqs, fee) - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInvalidPubKey) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeInvalidPubKey) acc2 = mapper.GetAccount(ctx, addr2) require.Nil(t, acc2.GetPubKey()) diff --git a/x/params/msg_status.go b/x/params/msg_status.go index 72704e4dc..7f9197c5c 100644 --- a/x/params/msg_status.go +++ b/x/params/msg_status.go @@ -24,7 +24,7 @@ func InitGenesis(ctx sdk.Context, k Keeper, data GenesisState) { // NewAnteHandler returns an AnteHandler that checks // whether msg type is activate or not func NewAnteHandler(k Keeper) sdk.AnteHandler { - return func(ctx sdk.Context, tx sdk.Tx) (sdk.Context, sdk.Result, bool) { + return func(ctx sdk.Context, tx sdk.Tx, simulate bool) (sdk.Context, sdk.Result, bool) { for _, msg := range tx.GetMsgs() { ok := k.Getter().GetBoolWithDefault(ctx, ActivatedParamKey(msg.Type()), false) if !ok { From 855222e8c343fa10e2ff46cf968614ae2e493625 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Mon, 27 Aug 2018 14:27:00 -0700 Subject: [PATCH 15/25] simulation: Allow operations to specify future operations The intent of this is to allow for simulating things like slashing for not voting on a governance proposal. To test this, you would queue all the validator votes in future blocks, and keep track of which ones you didn't slash. Then you could add queue a "check governance slashing operation" after the voting period is over. --- PENDING.md | 1 + cmd/gaia/app/sim_test.go | 6 +-- x/bank/simulation/msgs.go | 12 +++--- x/bank/simulation/sim_test.go | 2 +- x/gov/simulation/msgs.go | 19 +++++----- x/mock/simulation/random_simulate_blocks.go | 42 ++++++++++++++++++++- x/mock/simulation/types.go | 14 ++++++- x/slashing/simulation/msgs.go | 4 +- x/stake/simulation/msgs.go | 36 +++++++++--------- 9 files changed, 94 insertions(+), 42 deletions(-) diff --git a/PENDING.md b/PENDING.md index cee2ae14c..f413a1800 100644 --- a/PENDING.md +++ b/PENDING.md @@ -41,6 +41,7 @@ FEATURES * SDK * [querier] added custom querier functionality, so ABCI query requests can be handled by keepers + * [simulation] \#1924 allow operations to specify future operations * Tendermint diff --git a/cmd/gaia/app/sim_test.go b/cmd/gaia/app/sim_test.go index 9036c7e14..0ca936bdb 100644 --- a/cmd/gaia/app/sim_test.go +++ b/cmd/gaia/app/sim_test.go @@ -3,7 +3,6 @@ package app import ( "encoding/json" "flag" - "fmt" "math/rand" "testing" @@ -87,7 +86,7 @@ func appStateFn(r *rand.Rand, keys []crypto.PrivKey, accs []sdk.AccAddress) json func testAndRunTxs(app *GaiaApp) []simulation.Operation { return []simulation.Operation{ - banksim.TestAndRunSingleInputMsgSend(app.accountMapper), + banksim.SimulateSingleInputMsgSend(app.accountMapper), govsim.SimulateMsgSubmitProposal(app.govKeeper, app.stakeKeeper), govsim.SimulateMsgDeposit(app.govKeeper, app.stakeKeeper), govsim.SimulateMsgVote(app.govKeeper, app.stakeKeeper), @@ -171,11 +170,10 @@ func TestAppStateDeterminism(t *testing.T) { true, ) appHash := app.LastCommitID().Hash - fmt.Printf(">>> APP HASH: %v, %X\n", appHash, appHash) appHashList[j] = appHash } for k := 1; k < numTimesToRunPerSeed; k++ { - require.Equal(t, appHashList[0], appHashList[k]) + require.Equal(t, appHashList[0], appHashList[k], "appHash list: %v", appHashList) } } } diff --git a/x/bank/simulation/msgs.go b/x/bank/simulation/msgs.go index 633f6787f..e3f3ab77b 100644 --- a/x/bank/simulation/msgs.go +++ b/x/bank/simulation/msgs.go @@ -18,10 +18,10 @@ import ( "github.com/tendermint/tendermint/crypto" ) -// TestAndRunSingleInputMsgSend tests and runs a single msg send, with one input and one output, where both +// SimulateSingleInputMsgSend tests and runs a single msg send, with one input and one output, where both // accounts already exist. -func TestAndRunSingleInputMsgSend(mapper auth.AccountMapper) simulation.Operation { - return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { +func SimulateSingleInputMsgSend(mapper auth.AccountMapper) simulation.Operation { + return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, fOps []simulation.FutureOperation, err sdk.Error) { fromKey := simulation.RandomKey(r, keys) fromAddr := sdk.AccAddress(fromKey.PubKey().Address()) toKey := simulation.RandomKey(r, keys) @@ -36,13 +36,13 @@ func TestAndRunSingleInputMsgSend(mapper auth.AccountMapper) simulation.Operatio initFromCoins := mapper.GetAccount(ctx, fromAddr).GetCoins() if len(initFromCoins) == 0 { - return "skipping, no coins at all", nil + return "skipping, no coins at all", nil, nil } denomIndex := r.Intn(len(initFromCoins)) amt, goErr := randPositiveInt(r, initFromCoins[denomIndex].Amount) if goErr != nil { - return "skipping bank send due to account having no coins of denomination " + initFromCoins[denomIndex].Denom, nil + return "skipping bank send due to account having no coins of denomination " + initFromCoins[denomIndex].Denom, nil, nil } action = fmt.Sprintf("%s is sending %s %s to %s", @@ -61,7 +61,7 @@ func TestAndRunSingleInputMsgSend(mapper auth.AccountMapper) simulation.Operatio sendAndVerifyMsgSend(t, app, mapper, msg, ctx, log, []crypto.PrivKey{fromKey}) event("bank/sendAndVerifyMsgSend/ok") - return action, nil + return action, nil, nil } } diff --git a/x/bank/simulation/sim_test.go b/x/bank/simulation/sim_test.go index 88e7a5f3a..f61f72b4f 100644 --- a/x/bank/simulation/sim_test.go +++ b/x/bank/simulation/sim_test.go @@ -34,7 +34,7 @@ func TestBankWithRandomMessages(t *testing.T) { simulation.Simulate( t, mapp.BaseApp, appStateFn, []simulation.Operation{ - TestAndRunSingleInputMsgSend(mapper), + SimulateSingleInputMsgSend(mapper), }, []simulation.RandSetup{}, []simulation.Invariant{ diff --git a/x/gov/simulation/msgs.go b/x/gov/simulation/msgs.go index f270d3a7d..0b1530138 100644 --- a/x/gov/simulation/msgs.go +++ b/x/gov/simulation/msgs.go @@ -20,9 +20,10 @@ const ( denom = "steak" ) -// SimulateMsgSubmitProposal +// SimulateMsgSubmitProposal simulates a msg Submit Proposal +// Note: Currently doesn't ensure that the proposal txt is in JSON form func SimulateMsgSubmitProposal(k gov.Keeper, sk stake.Keeper) simulation.Operation { - return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, fOps []simulation.FutureOperation, err sdk.Error) { key := simulation.RandomKey(r, keys) addr := sdk.AccAddress(key.PubKey().Address()) deposit := randomDeposit(r) @@ -45,18 +46,18 @@ func SimulateMsgSubmitProposal(k gov.Keeper, sk stake.Keeper) simulation.Operati } event(fmt.Sprintf("gov/MsgSubmitProposal/%v", result.IsOK())) action = fmt.Sprintf("TestMsgSubmitProposal: ok %v, msg %s", result.IsOK(), msg.GetSignBytes()) - return action, nil + return action, nil, nil } } // SimulateMsgDeposit func SimulateMsgDeposit(k gov.Keeper, sk stake.Keeper) simulation.Operation { - return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, fOp []simulation.FutureOperation, err sdk.Error) { key := simulation.RandomKey(r, keys) addr := sdk.AccAddress(key.PubKey().Address()) proposalID, ok := randomProposalID(r, k, ctx) if !ok { - return "no-operation", nil + return "no-operation", nil, nil } deposit := randomDeposit(r) msg := gov.NewMsgDeposit(addr, proposalID, deposit) @@ -72,18 +73,18 @@ func SimulateMsgDeposit(k gov.Keeper, sk stake.Keeper) simulation.Operation { } event(fmt.Sprintf("gov/MsgDeposit/%v", result.IsOK())) action = fmt.Sprintf("TestMsgDeposit: ok %v, msg %s", result.IsOK(), msg.GetSignBytes()) - return action, nil + return action, nil, nil } } // SimulateMsgVote func SimulateMsgVote(k gov.Keeper, sk stake.Keeper) simulation.Operation { - return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, fOp []simulation.FutureOperation, err sdk.Error) { key := simulation.RandomKey(r, keys) addr := sdk.AccAddress(key.PubKey().Address()) proposalID, ok := randomProposalID(r, k, ctx) if !ok { - return "no-operation", nil + return "no-operation", nil, nil } option := randomVotingOption(r) msg := gov.NewMsgVote(addr, proposalID, option) @@ -95,7 +96,7 @@ func SimulateMsgVote(k gov.Keeper, sk stake.Keeper) simulation.Operation { } event(fmt.Sprintf("gov/MsgVote/%v", result.IsOK())) action = fmt.Sprintf("TestMsgVote: ok %v, msg %s", result.IsOK(), msg.GetSignBytes()) - return action, nil + return action, nil, nil } } diff --git a/x/mock/simulation/random_simulate_blocks.go b/x/mock/simulation/random_simulate_blocks.go index 5b1d4030c..995013ef8 100644 --- a/x/mock/simulation/random_simulate_blocks.go +++ b/x/mock/simulation/random_simulate_blocks.go @@ -70,6 +70,8 @@ func SimulateFromSeed( request := abci.RequestBeginBlock{Header: header} var pastTimes []time.Time + // These are operations which have been queued by previous operations + operationQueue := make(map[int][]Operation) for i := 0; i < numBlocks; i++ { @@ -96,9 +98,14 @@ func SimulateFromSeed( default: thisBlockSize = r.Intn(blockSize * 4) } + // Run queued operations. Ignores blocksize if blocksize is too small + log, numQueuedOpsRan := runQueuedOperations(operationQueue, int(header.Height), t, r, app, ctx, keys, log, event) + opCount += numQueuedOpsRan + thisBlockSize -= numQueuedOpsRan for j := 0; j < thisBlockSize; j++ { - logUpdate, err := ops[r.Intn(len(ops))](t, r, app, ctx, keys, log, event) + logUpdate, futureOps, err := ops[r.Intn(len(ops))](t, r, app, ctx, keys, log, event) log += "\n" + logUpdate + queueOperations(operationQueue, futureOps) require.Nil(t, err, log) if onOperation { @@ -134,6 +141,39 @@ func SimulateFromSeed( DisplayEvents(events) } +// adds all future operations into the operation queue. +func queueOperations(queuedOperations map[int][]Operation, futureOperations []FutureOperation) { + if futureOperations == nil { + return + } + for _, futureOp := range futureOperations { + if val, ok := queuedOperations[futureOp.BlockHeight]; ok { + queuedOperations[futureOp.BlockHeight] = append(val, futureOp.Op) + } else { + queuedOperations[futureOp.BlockHeight] = []Operation{futureOp.Op} + } + } +} + +func runQueuedOperations(queueOperations map[int][]Operation, height int, t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, + privKeys []crypto.PrivKey, log string, event func(string)) (updatedLog string, numOpsRan int) { + updatedLog = log + if queuedOps, ok := queueOperations[height]; ok { + numOps := len(queuedOps) + for i := 0; i < numOps; i++ { + // For now, queued operations cannot queue more operations. + // If a need arises for us to support queued messages to queue more messages, this can + // be changed. + logUpdate, _, err := queuedOps[i](t, r, app, ctx, privKeys, updatedLog, event) + updatedLog += "\n" + logUpdate + require.Nil(t, err, updatedLog) + } + delete(queueOperations, height) + return updatedLog, numOps + } + return log, 0 +} + func getKeys(validators map[string]mockValidator) []string { keys := make([]string, len(validators)) i := 0 diff --git a/x/mock/simulation/types.go b/x/mock/simulation/types.go index 3ece3c2e9..2516e07ae 100644 --- a/x/mock/simulation/types.go +++ b/x/mock/simulation/types.go @@ -19,10 +19,13 @@ type ( // For ease of debugging, // an operation returns a descriptive message "action", // which details what this fuzzed state machine transition actually did. + // + // Operations can optionally provide a list of "FutureOperations" to run later + // These will be ran at the beginning of the corresponding block. Operation func( t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, privKeys []crypto.PrivKey, log string, event func(string), - ) (action string, err sdk.Error) + ) (action string, futureOperations []FutureOperation, err sdk.Error) // RandSetup performs the random setup the mock module needs. RandSetup func(r *rand.Rand, privKeys []crypto.PrivKey) @@ -36,6 +39,15 @@ type ( val abci.Validator livenessState int } + + // FutureOperation is an operation which will be ran at the + // beginning of the provided BlockHeight. + // In the (likely) event that multiple operations are queued at the same + // block height, they will execute in a FIFO pattern. + FutureOperation struct { + BlockHeight int + Op Operation + } ) // PeriodicInvariant returns an Invariant function closure that asserts diff --git a/x/slashing/simulation/msgs.go b/x/slashing/simulation/msgs.go index e694bd1d7..e4900889e 100644 --- a/x/slashing/simulation/msgs.go +++ b/x/slashing/simulation/msgs.go @@ -17,7 +17,7 @@ import ( // SimulateMsgUnjail func SimulateMsgUnjail(k slashing.Keeper) simulation.Operation { - return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, fOp []simulation.FutureOperation, err sdk.Error) { key := simulation.RandomKey(r, keys) address := sdk.AccAddress(key.PubKey().Address()) msg := slashing.NewMsgUnjail(address) @@ -29,6 +29,6 @@ func SimulateMsgUnjail(k slashing.Keeper) simulation.Operation { } event(fmt.Sprintf("slashing/MsgUnjail/%v", result.IsOK())) action = fmt.Sprintf("TestMsgUnjail: ok %v, msg %s", result.IsOK(), msg.GetSignBytes()) - return action, nil + return action, nil, nil } } diff --git a/x/stake/simulation/msgs.go b/x/stake/simulation/msgs.go index 995e4f358..4280e70c1 100644 --- a/x/stake/simulation/msgs.go +++ b/x/stake/simulation/msgs.go @@ -19,7 +19,7 @@ import ( // SimulateMsgCreateValidator func SimulateMsgCreateValidator(m auth.AccountMapper, k stake.Keeper) simulation.Operation { - return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, fOp []simulation.FutureOperation, err sdk.Error) { denom := k.GetParams(ctx).BondDenom description := stake.Description{ Moniker: simulation.RandStringOfLength(r, 10), @@ -32,7 +32,7 @@ func SimulateMsgCreateValidator(m auth.AccountMapper, k stake.Keeper) simulation amount = simulation.RandomAmount(r, amount) } if amount.Equal(sdk.ZeroInt()) { - return "no-operation", nil + return "no-operation", nil, nil } msg := stake.MsgCreateValidator{ Description: description, @@ -50,13 +50,13 @@ func SimulateMsgCreateValidator(m auth.AccountMapper, k stake.Keeper) simulation event(fmt.Sprintf("stake/MsgCreateValidator/%v", result.IsOK())) // require.True(t, result.IsOK(), "expected OK result but instead got %v", result) action = fmt.Sprintf("TestMsgCreateValidator: ok %v, msg %s", result.IsOK(), msg.GetSignBytes()) - return action, nil + return action, nil, nil } } // SimulateMsgEditValidator func SimulateMsgEditValidator(k stake.Keeper) simulation.Operation { - return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, fOp []simulation.FutureOperation, err sdk.Error) { description := stake.Description{ Moniker: simulation.RandStringOfLength(r, 10), Identity: simulation.RandStringOfLength(r, 10), @@ -78,13 +78,13 @@ func SimulateMsgEditValidator(k stake.Keeper) simulation.Operation { } event(fmt.Sprintf("stake/MsgEditValidator/%v", result.IsOK())) action = fmt.Sprintf("TestMsgEditValidator: ok %v, msg %s", result.IsOK(), msg.GetSignBytes()) - return action, nil + return action, nil, nil } } // SimulateMsgDelegate func SimulateMsgDelegate(m auth.AccountMapper, k stake.Keeper) simulation.Operation { - return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, fOp []simulation.FutureOperation, err sdk.Error) { denom := k.GetParams(ctx).BondDenom validatorKey := simulation.RandomKey(r, keys) validatorAddress := sdk.AccAddress(validatorKey.PubKey().Address()) @@ -95,7 +95,7 @@ func SimulateMsgDelegate(m auth.AccountMapper, k stake.Keeper) simulation.Operat amount = simulation.RandomAmount(r, amount) } if amount.Equal(sdk.ZeroInt()) { - return "no-operation", nil + return "no-operation", nil, nil } msg := stake.MsgDelegate{ DelegatorAddr: delegatorAddress, @@ -110,13 +110,13 @@ func SimulateMsgDelegate(m auth.AccountMapper, k stake.Keeper) simulation.Operat } event(fmt.Sprintf("stake/MsgDelegate/%v", result.IsOK())) action = fmt.Sprintf("TestMsgDelegate: ok %v, msg %s", result.IsOK(), msg.GetSignBytes()) - return action, nil + return action, nil, nil } } // SimulateMsgBeginUnbonding func SimulateMsgBeginUnbonding(m auth.AccountMapper, k stake.Keeper) simulation.Operation { - return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, fOp []simulation.FutureOperation, err sdk.Error) { denom := k.GetParams(ctx).BondDenom validatorKey := simulation.RandomKey(r, keys) validatorAddress := sdk.AccAddress(validatorKey.PubKey().Address()) @@ -127,7 +127,7 @@ func SimulateMsgBeginUnbonding(m auth.AccountMapper, k stake.Keeper) simulation. amount = simulation.RandomAmount(r, amount) } if amount.Equal(sdk.ZeroInt()) { - return "no-operation", nil + return "no-operation", nil, nil } msg := stake.MsgBeginUnbonding{ DelegatorAddr: delegatorAddress, @@ -142,13 +142,13 @@ func SimulateMsgBeginUnbonding(m auth.AccountMapper, k stake.Keeper) simulation. } event(fmt.Sprintf("stake/MsgBeginUnbonding/%v", result.IsOK())) action = fmt.Sprintf("TestMsgBeginUnbonding: ok %v, msg %s", result.IsOK(), msg.GetSignBytes()) - return action, nil + return action, nil, nil } } // SimulateMsgCompleteUnbonding func SimulateMsgCompleteUnbonding(k stake.Keeper) simulation.Operation { - return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, fOp []simulation.FutureOperation, err sdk.Error) { validatorKey := simulation.RandomKey(r, keys) validatorAddress := sdk.AccAddress(validatorKey.PubKey().Address()) delegatorKey := simulation.RandomKey(r, keys) @@ -165,13 +165,13 @@ func SimulateMsgCompleteUnbonding(k stake.Keeper) simulation.Operation { } event(fmt.Sprintf("stake/MsgCompleteUnbonding/%v", result.IsOK())) action = fmt.Sprintf("TestMsgCompleteUnbonding: ok %v, msg %s", result.IsOK(), msg.GetSignBytes()) - return action, nil + return action, nil, nil } } // SimulateMsgBeginRedelegate func SimulateMsgBeginRedelegate(m auth.AccountMapper, k stake.Keeper) simulation.Operation { - return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, fOp []simulation.FutureOperation, err sdk.Error) { denom := k.GetParams(ctx).BondDenom sourceValidatorKey := simulation.RandomKey(r, keys) sourceValidatorAddress := sdk.AccAddress(sourceValidatorKey.PubKey().Address()) @@ -185,7 +185,7 @@ func SimulateMsgBeginRedelegate(m auth.AccountMapper, k stake.Keeper) simulation amount = simulation.RandomAmount(r, amount) } if amount.Equal(sdk.ZeroInt()) { - return "no-operation", nil + return "no-operation", nil, nil } msg := stake.MsgBeginRedelegate{ DelegatorAddr: delegatorAddress, @@ -201,13 +201,13 @@ func SimulateMsgBeginRedelegate(m auth.AccountMapper, k stake.Keeper) simulation } event(fmt.Sprintf("stake/MsgBeginRedelegate/%v", result.IsOK())) action = fmt.Sprintf("TestMsgBeginRedelegate: %s", msg.GetSignBytes()) - return action, nil + return action, nil, nil } } // SimulateMsgCompleteRedelegate func SimulateMsgCompleteRedelegate(k stake.Keeper) simulation.Operation { - return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, fOp []simulation.FutureOperation, err sdk.Error) { validatorSrcKey := simulation.RandomKey(r, keys) validatorSrcAddress := sdk.AccAddress(validatorSrcKey.PubKey().Address()) validatorDstKey := simulation.RandomKey(r, keys) @@ -227,7 +227,7 @@ func SimulateMsgCompleteRedelegate(k stake.Keeper) simulation.Operation { } event(fmt.Sprintf("stake/MsgCompleteRedelegate/%v", result.IsOK())) action = fmt.Sprintf("TestMsgCompleteRedelegate: ok %v, msg %s", result.IsOK(), msg.GetSignBytes()) - return action, nil + return action, nil, nil } } From bfdeb3f8ce33d08828dd2e0f0822c3006d6751cc Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Mon, 27 Aug 2018 15:18:01 -0700 Subject: [PATCH 16/25] works --- x/gov/client/cli/tx.go | 44 ++++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/x/gov/client/cli/tx.go b/x/gov/client/cli/tx.go index 26883b9c3..65c922c6c 100644 --- a/x/gov/client/cli/tx.go +++ b/x/gov/client/cli/tx.go @@ -244,7 +244,7 @@ func GetCmdVote(cdc *wire.Codec) *cobra.Command { } // GetCmdQueryProposal implements the query proposal command. -func GetCmdQueryProposal(storeName string, cdc *wire.Codec) *cobra.Command { +func GetCmdQueryProposal(queryRoute string, cdc *wire.Codec) *cobra.Command { cmd := &cobra.Command{ Use: "query-proposal", Short: "query proposal details", @@ -261,7 +261,7 @@ func GetCmdQueryProposal(storeName string, cdc *wire.Codec) *cobra.Command { return err } - res, err := cliCtx.QueryWithData("custom/gov/proposal", bz) + res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/proposal", queryRoute), bz) if err != nil { return err } @@ -278,7 +278,7 @@ func GetCmdQueryProposal(storeName string, cdc *wire.Codec) *cobra.Command { // nolint: gocyclo // GetCmdQueryProposals implements a query proposals command. -func GetCmdQueryProposals(storeName string, cdc *wire.Codec) *cobra.Command { +func GetCmdQueryProposals(queryRoute string, cdc *wire.Codec) *cobra.Command { cmd := &cobra.Command{ Use: "query-proposals", Short: "query proposals with optional filters", @@ -323,12 +323,26 @@ func GetCmdQueryProposals(storeName string, cdc *wire.Codec) *cobra.Command { cliCtx := context.NewCLIContext().WithCodec(cdc) - res, err := cliCtx.QueryWithData("custom/gov/proposals", bz) + res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/proposals", queryRoute), bz) if err != nil { return err } - fmt.Println(string(res)) + 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 }, } @@ -343,7 +357,7 @@ func GetCmdQueryProposals(storeName string, cdc *wire.Codec) *cobra.Command { // Command to Get a Proposal Information // GetCmdQueryVote implements the query proposal vote command. -func GetCmdQueryVote(storeName string, cdc *wire.Codec) *cobra.Command { +func GetCmdQueryVote(queryRoute string, cdc *wire.Codec) *cobra.Command { cmd := &cobra.Command{ Use: "query-vote", Short: "query vote", @@ -365,7 +379,7 @@ func GetCmdQueryVote(storeName string, cdc *wire.Codec) *cobra.Command { return err } - res, err := cliCtx.QueryWithData("custom/gov/vote", bz) + res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/vote", queryRoute), bz) if err != nil { return err } @@ -382,7 +396,7 @@ func GetCmdQueryVote(storeName string, cdc *wire.Codec) *cobra.Command { } // GetCmdQueryVotes implements the command to query for proposal votes. -func GetCmdQueryVotes(storeName string, cdc *wire.Codec) *cobra.Command { +func GetCmdQueryVotes(queryRoute string, cdc *wire.Codec) *cobra.Command { cmd := &cobra.Command{ Use: "query-votes", Short: "query votes on a proposal", @@ -398,7 +412,7 @@ func GetCmdQueryVotes(storeName string, cdc *wire.Codec) *cobra.Command { return err } - res, err := cliCtx.QueryWithData("custom/gov/votes", bz) + res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/votes", queryRoute), bz) if err != nil { return err } @@ -415,7 +429,7 @@ func GetCmdQueryVotes(storeName string, cdc *wire.Codec) *cobra.Command { // Command to Get a specific Deposit Information // GetCmdQueryDeposit implements the query proposal deposit command. -func GetCmdQueryDeposit(storeName string, cdc *wire.Codec) *cobra.Command { +func GetCmdQueryDeposit(queryRoute string, cdc *wire.Codec) *cobra.Command { cmd := &cobra.Command{ Use: "query-deposit", Short: "query deposit", @@ -437,7 +451,7 @@ func GetCmdQueryDeposit(storeName string, cdc *wire.Codec) *cobra.Command { return err } - res, err := cliCtx.QueryWithData("custom/gov/deposit", bz) + res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/deposit", queryRoute), bz) if err != nil { return err } @@ -454,7 +468,7 @@ func GetCmdQueryDeposit(storeName string, cdc *wire.Codec) *cobra.Command { } // GetCmdQueryDeposits implements the command to query for proposal deposits. -func GetCmdQueryDeposits(storeName string, cdc *wire.Codec) *cobra.Command { +func GetCmdQueryDeposits(queryRoute string, cdc *wire.Codec) *cobra.Command { cmd := &cobra.Command{ Use: "query-deposits", Short: "query deposits on a proposal", @@ -470,7 +484,7 @@ func GetCmdQueryDeposits(storeName string, cdc *wire.Codec) *cobra.Command { return err } - res, err := cliCtx.QueryWithData("custom/gov/deposits", bz) + res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/deposits", queryRoute), bz) if err != nil { return err } @@ -486,7 +500,7 @@ func GetCmdQueryDeposits(storeName string, cdc *wire.Codec) *cobra.Command { } // GetCmdQueryDeposits implements the command to query for proposal deposits. -func GetCmdQueryTally(storeName string, cdc *wire.Codec) *cobra.Command { +func GetCmdQueryTally(queryRoute string, cdc *wire.Codec) *cobra.Command { cmd := &cobra.Command{ Use: "query-tally", Short: "get the tally of a proposal vote", @@ -502,7 +516,7 @@ func GetCmdQueryTally(storeName string, cdc *wire.Codec) *cobra.Command { return err } - res, err := cliCtx.QueryWithData("custom/gov/tally", bz) + res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/tally", queryRoute), bz) if err != nil { return err } From fc20f757ecb0761de17c189de4380a0feea405e0 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 27 Aug 2018 18:42:03 -0400 Subject: [PATCH 17/25] Load ledger device at runtime --- crypto/ledger.go | 17 ++++++++++------- crypto/ledger_secp256k1.go | 31 ++++++++++++++++++------------- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/crypto/ledger.go b/crypto/ledger.go index 9d446202f..5489810bc 100644 --- a/crypto/ledger.go +++ b/crypto/ledger.go @@ -6,13 +6,16 @@ import ( ledger "github.com/zondax/ledger-goclient" ) -// If ledger support (build tag) has been enabled, automically attempt to load -// and set the ledger device, ledgerDevice, if it has not already been set. +// If ledger support (build tag) has been enabled, set the DiscoverLedger +// function which is responsible for loading the Ledger device at runtime or +// returning an error. func init() { - device, err := ledger.FindLedger() - if err != nil { - ledgerDeviceErr = err - } else { - ledgerDevice = device + DiscoverLedger = func() (LedgerSECP256K1, error) { + device, err := ledger.FindLedger() + if err != nil { + return nil, err + } + + return device, nil } } diff --git a/crypto/ledger_secp256k1.go b/crypto/ledger_secp256k1.go index 8cb175d4f..5f6bcb1ad 100644 --- a/crypto/ledger_secp256k1.go +++ b/crypto/ledger_secp256k1.go @@ -1,24 +1,26 @@ package crypto import ( - "errors" "fmt" + "github.com/pkg/errors" + secp256k1 "github.com/btcsuite/btcd/btcec" tmcrypto "github.com/tendermint/tendermint/crypto" tmsecp256k1 "github.com/tendermint/tendermint/crypto/secp256k1" ) var ( - ledgerDevice LedgerSECP256K1 - ledgerDeviceErr error - - // ErrMissingLedgerDevice is used to reflect that a ledger device load has - // not been attempted. - ErrMissingLedgerDevice = errors.New("missing ledger device") + // DiscoverLedger defines a function to be invoked at runtime for discovering + // a connected Ledger device. + DiscoverLedger DiscoverLedgerFn ) type ( + // DiscoverLedgerFn defines a Ledger discovery function that returns a + // connected device or an error upon failure. + DiscoverLedgerFn func() (LedgerSECP256K1, error) + // DerivationPath represents a Ledger derivation path. DerivationPath []uint32 @@ -47,18 +49,21 @@ type ( // CONTRACT: The ledger device, ledgerDevice, must be loaded and set prior to // any creation of a PrivKeyLedgerSecp256k1. func NewPrivKeyLedgerSecp256k1(path DerivationPath) (tmcrypto.PrivKey, error) { - if ledgerDevice == nil { - err := ErrMissingLedgerDevice - if ledgerDeviceErr != nil { - err = ledgerDeviceErr + var ledgerDevice LedgerSECP256K1 + + if DiscoverLedger != nil { + device, err := DiscoverLedger() + if err != nil { + return nil, errors.Wrap(err, "failed to create PrivKeyLedgerSecp256k1") } - return nil, fmt.Errorf("failed to create PrivKeyLedgerSecp256k1: %v", err) + ledgerDevice = device + } else { + return nil, errors.New("no ledger discovery function defined") } pkl := &PrivKeyLedgerSecp256k1{Path: path, ledger: ledgerDevice} - // cache the pubkey for later use pubKey, err := pkl.getPubKey() if err != nil { return nil, err From 0245a4b5ee6cd0a85f037b83c96325a147862112 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 27 Aug 2018 18:45:34 -0400 Subject: [PATCH 18/25] Update pending log --- PENDING.md | 2 ++ crypto/ledger_secp256k1.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/PENDING.md b/PENDING.md index cee2ae14c..11f718031 100644 --- a/PENDING.md +++ b/PENDING.md @@ -79,5 +79,7 @@ BUG FIXES * SDK * \#1988 Make us compile on OpenBSD (disable ledger) [#1988] (https://github.com/cosmos/cosmos-sdk/issues/1988) * \#2105 Fix DB Iterator leak, which may leak a go routine. + * [ledger] \#2064 Fix inability to sign and send transactions via the LCD by + loading a Ledger device at runtime. * Tendermint diff --git a/crypto/ledger_secp256k1.go b/crypto/ledger_secp256k1.go index 5f6bcb1ad..b29570179 100644 --- a/crypto/ledger_secp256k1.go +++ b/crypto/ledger_secp256k1.go @@ -59,7 +59,7 @@ func NewPrivKeyLedgerSecp256k1(path DerivationPath) (tmcrypto.PrivKey, error) { ledgerDevice = device } else { - return nil, errors.New("no ledger discovery function defined") + return nil, errors.New("no Ledger discovery function defined") } pkl := &PrivKeyLedgerSecp256k1{Path: path, ledger: ledgerDevice} From de061fa8e195f41b7669556761fc26cea00c40f7 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 27 Aug 2018 18:57:06 -0400 Subject: [PATCH 19/25] Do not export ledger discovery types --- crypto/ledger.go | 4 ++-- crypto/ledger_secp256k1.go | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crypto/ledger.go b/crypto/ledger.go index 5489810bc..346d4bfb3 100644 --- a/crypto/ledger.go +++ b/crypto/ledger.go @@ -6,11 +6,11 @@ import ( ledger "github.com/zondax/ledger-goclient" ) -// If ledger support (build tag) has been enabled, set the DiscoverLedger +// If ledger support (build tag) has been enabled, set the discoverLedger // function which is responsible for loading the Ledger device at runtime or // returning an error. func init() { - DiscoverLedger = func() (LedgerSECP256K1, error) { + discoverLedger = func() (LedgerSECP256K1, error) { device, err := ledger.FindLedger() if err != nil { return nil, err diff --git a/crypto/ledger_secp256k1.go b/crypto/ledger_secp256k1.go index b29570179..c59f8e3b4 100644 --- a/crypto/ledger_secp256k1.go +++ b/crypto/ledger_secp256k1.go @@ -11,15 +11,15 @@ import ( ) var ( - // DiscoverLedger defines a function to be invoked at runtime for discovering + // discoverLedger defines a function to be invoked at runtime for discovering // a connected Ledger device. - DiscoverLedger DiscoverLedgerFn + discoverLedger discoverLedgerFn ) type ( - // DiscoverLedgerFn defines a Ledger discovery function that returns a + // discoverLedgerFn defines a Ledger discovery function that returns a // connected device or an error upon failure. - DiscoverLedgerFn func() (LedgerSECP256K1, error) + discoverLedgerFn func() (LedgerSECP256K1, error) // DerivationPath represents a Ledger derivation path. DerivationPath []uint32 @@ -51,8 +51,8 @@ type ( func NewPrivKeyLedgerSecp256k1(path DerivationPath) (tmcrypto.PrivKey, error) { var ledgerDevice LedgerSECP256K1 - if DiscoverLedger != nil { - device, err := DiscoverLedger() + if discoverLedger != nil { + device, err := discoverLedger() if err != nil { return nil, errors.Wrap(err, "failed to create PrivKeyLedgerSecp256k1") } From 1a6c4785e9b824a2f229050eb3644d951fb03716 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 28 Aug 2018 08:16:50 -0400 Subject: [PATCH 20/25] Cleanup ledger godocs and reduce complexity --- crypto/ledger.go | 6 +++--- crypto/ledger_secp256k1.go | 18 +++++++----------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/crypto/ledger.go b/crypto/ledger.go index 346d4bfb3..b9aa65b56 100644 --- a/crypto/ledger.go +++ b/crypto/ledger.go @@ -6,9 +6,9 @@ import ( ledger "github.com/zondax/ledger-goclient" ) -// If ledger support (build tag) has been enabled, set the discoverLedger -// function which is responsible for loading the Ledger device at runtime or -// returning an error. +// If ledger support (build tag) has been enabled, which implies a CGO dependency, +// set the discoverLedger function which is responsible for loading the Ledger +// device at runtime or returning an error. func init() { discoverLedger = func() (LedgerSECP256K1, error) { device, err := ledger.FindLedger() diff --git a/crypto/ledger_secp256k1.go b/crypto/ledger_secp256k1.go index c59f8e3b4..49f110b3d 100644 --- a/crypto/ledger_secp256k1.go +++ b/crypto/ledger_secp256k1.go @@ -49,20 +49,16 @@ type ( // CONTRACT: The ledger device, ledgerDevice, must be loaded and set prior to // any creation of a PrivKeyLedgerSecp256k1. func NewPrivKeyLedgerSecp256k1(path DerivationPath) (tmcrypto.PrivKey, error) { - var ledgerDevice LedgerSECP256K1 - - if discoverLedger != nil { - device, err := discoverLedger() - if err != nil { - return nil, errors.Wrap(err, "failed to create PrivKeyLedgerSecp256k1") - } - - ledgerDevice = device - } else { + if discoverLedger == nil { return nil, errors.New("no Ledger discovery function defined") } - pkl := &PrivKeyLedgerSecp256k1{Path: path, ledger: ledgerDevice} + device, err := discoverLedger() + if err != nil { + return nil, errors.Wrap(err, "failed to create PrivKeyLedgerSecp256k1") + } + + pkl := &PrivKeyLedgerSecp256k1{Path: path, ledger: device} pubKey, err := pkl.getPubKey() if err != nil { From bdbdd1a8323f14b27b4d5d6f5cf879282a326658 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 28 Aug 2018 08:24:19 -0400 Subject: [PATCH 21/25] Further godoc updates --- crypto/ledger_secp256k1.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crypto/ledger_secp256k1.go b/crypto/ledger_secp256k1.go index 49f110b3d..ff05d31ba 100644 --- a/crypto/ledger_secp256k1.go +++ b/crypto/ledger_secp256k1.go @@ -18,7 +18,8 @@ var ( type ( // discoverLedgerFn defines a Ledger discovery function that returns a - // connected device or an error upon failure. + // connected device or an error upon failure. Its allows a method to avoid CGO + // dependencies when Ledger support is potentially not enabled. discoverLedgerFn func() (LedgerSECP256K1, error) // DerivationPath represents a Ledger derivation path. From e89e860524c76025992b99787da44e28444ae8b0 Mon Sep 17 00:00:00 2001 From: Fabian Date: Tue, 28 Aug 2018 16:18:06 +0200 Subject: [PATCH 22/25] Update config.js --- docs/config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/config.js b/docs/config.js index 42df9de92..93426bcc5 100644 --- a/docs/config.js +++ b/docs/config.js @@ -2,7 +2,7 @@ module.exports = { title: "Cosmos Network", description: "Documentation for the Cosmos Network.", dest: "./dist/docs", - base: "/", + base: "/docs/", markdown: { lineNumbers: true }, From 946e24d7bfce0ebbe72ffde6dd2d1a382e76e96e Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Tue, 28 Aug 2018 09:53:56 -0700 Subject: [PATCH 23/25] Merge PR #2152: Make CI not update the lock file * CI: Make CI not update the lock file We want CI to be running the lock in the repo, not generating a new one. Linting now ensures that the lock file is up to date. * Switch to Chris' comment * Update pending to indicate this new command --- Makefile | 5 +++++ PENDING.md | 1 + 2 files changed, 6 insertions(+) diff --git a/Makefile b/Makefile index 66d566309..983234dbc 100644 --- a/Makefile +++ b/Makefile @@ -106,6 +106,11 @@ get_dev_tools: cd tools && $(MAKE) get_dev_tools get_vendor_deps: + @echo "--> Generating vendor directory via dep ensure" + @rm -rf .vendor-new + @dep ensure -v -vendor-only + +update_vendor_deps: @echo "--> Running dep ensure" @rm -rf .vendor-new @dep ensure -v diff --git a/PENDING.md b/PENDING.md index d65d34d8a..dd18be3c0 100644 --- a/PENDING.md +++ b/PENDING.md @@ -12,6 +12,7 @@ BREAKING CHANGES * [cli] \#1983 you can now pass --pubkey or --address to gaiacli keys show to return a plaintext representation of the key's address or public key for use with other commands * [cli] \#2061 changed proposalID in governance REST endpoints to proposal-id * [cli] \#2014 `gaiacli advanced` no longer exists - to access `ibc`, `rest-server`, and `validator-set` commands use `gaiacli ibc`, `gaiacli rest-server`, and `gaiacli tendermint`, respectively + * [makefile] `get_vendor_deps` no longer updates lock file it just updates vendor directory. Use `update_vendor_deps` to update the lock file. [#2152](https://github.com/cosmos/cosmos-sdk/pull/2152) * Gaia * Make the transient store key use a distinct store key. [#2013](https://github.com/cosmos/cosmos-sdk/pull/2013) From c6d692e27fa19df8ae4d4b0206cd803c61f7a3f4 Mon Sep 17 00:00:00 2001 From: Matthew Slipper Date: Tue, 28 Aug 2018 09:59:42 -0700 Subject: [PATCH 24/25] Merge PR #2172: Use cobra.NoArgs where appropriate Closes #885. --- x/slashing/client/cli/tx.go | 2 +- x/stake/client/cli/query.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x/slashing/client/cli/tx.go b/x/slashing/client/cli/tx.go index 5831ef310..2fdd65c6c 100644 --- a/x/slashing/client/cli/tx.go +++ b/x/slashing/client/cli/tx.go @@ -18,7 +18,7 @@ import ( func GetCmdUnjail(cdc *wire.Codec) *cobra.Command { cmd := &cobra.Command{ Use: "unjail", - Args: cobra.ExactArgs(0), + Args: cobra.NoArgs, Short: "unjail validator previously jailed for downtime", RunE: func(cmd *cobra.Command, args []string) error { txCtx := authctx.NewTxContextFromCLI().WithCodec(cdc) diff --git a/x/stake/client/cli/query.go b/x/stake/client/cli/query.go index 4b8e99e7d..d58ea3938 100644 --- a/x/stake/client/cli/query.go +++ b/x/stake/client/cli/query.go @@ -423,7 +423,7 @@ func GetCmdQueryPool(storeName string, cdc *wire.Codec) *cobra.Command { cmd := &cobra.Command{ Use: "pool", Short: "Query the current staking pool values", - Args: cobra.ExactArgs(0), + Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { key := stake.PoolKey cliCtx := context.NewCLIContext().WithCodec(cdc) @@ -462,7 +462,7 @@ func GetCmdQueryParams(storeName string, cdc *wire.Codec) *cobra.Command { cmd := &cobra.Command{ Use: "parameters", Short: "Query the current staking parameters information", - Args: cobra.ExactArgs(0), + Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { key := stake.ParamKey cliCtx := context.NewCLIContext().WithCodec(cdc) From dfd29a523cffb2320174ee05a8580f6ba308f756 Mon Sep 17 00:00:00 2001 From: Jack Zampolin Date: Tue, 28 Aug 2018 11:13:21 -0700 Subject: [PATCH 25/25] Update PRIORIEIES.md --- docs/PRIORITIES.md | 92 +++++++++++++++++++++++++++++----------------- 1 file changed, 58 insertions(+), 34 deletions(-) diff --git a/docs/PRIORITIES.md b/docs/PRIORITIES.md index 631d9b9ea..d5e425776 100644 --- a/docs/PRIORITIES.md +++ b/docs/PRIORITIES.md @@ -4,48 +4,72 @@ - Collection - Simple flat fee set in-config by validators & full nodes - ref [#1921](https://github.com/cosmos/cosmos-sdk/issues/1921) + - @sunnya97 working on implementation + - _*BLOCKER:*_ Blocked on [tendermint/tendermint#2275](https://github.com/tendermint/tendermint/issues/2275) @ValarDragon - Distribution - "Piggy bank" fee distribution - ref [#1944](https://github.com/cosmos/cosmos-sdk/pull/1944) (spec) -- Reserve pool - - Collects in a special field for now, no spending + - @rigelrozanski working on implementation +- EST TIMELINE: + - Work on fees should be completed in the `v0.25.0` release + +## Staking/Slashing/Stability + +- Unbonding state for validators (WIP) [#2163](https://github.com/cosmos/cosmos-sdk/pull/2163) @rigelrozanski + - Needs :eyes: from @chris + - Should be in `v0.25.0` release +- Slashing period PR - ref [#2122](https://github.com/cosmos/cosmos-sdk/pull/2122) + - Needs :eyes: from @cwgoes and @jaekwon + - Should be in `v0.25.0` release +- Other slashing issues blocking for launch - [#1256](https://github.com/cosmos/cosmos-sdk/issues/1256) +- Update staking/slashing for NextValSet change + - @cwgoes to start next +- Miscellaneous minor staking issues + - [List here](https://github.com/cosmos/cosmos-sdk/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+label%3Astaking+label%3Aprelaunch) + - Need to figure out scope of work here to estimate time + - @rigelrozanski to start next + +## Vesting + +- Single `VestingAccount` allowing delegation/voting but no withdrawals +- Ref [#1875](https://github.com/cosmos/cosmos-sdk/pull/1875) (spec) +- @AdityaSripal working on this. + - Should be in `v0.25.0` release + +## Multisig + +- Already implemented on TM side, need simple CLI interface +- @alessio working on the SDK side of things here +- Need to schedule some time with @alessio, @ebuchman and @ValarDragon this week to finalize feature set/implementation plan + +## ABCI Changes + +- Need to update for new ABCI changes - error string, tags are list of lists, proposer in header (Tendermint 0.24?) +- @cwgoes has done some work here. Should be on `develop` in tendermint w/in next week. +- Include in tendermint `v0.24.0` release? + +## Gas + +- Simple transaction benchmarking work by @jlandrews to inform additional work here +- Integrate @alessio's simulation work into CLI and LCD +- Sanity Checks + +## LCD + +- Bianje working on implementation ([#2147](https://github.com/cosmos/cosmos-sdk/pull/2147)) + - ICS 0,ICS 1, ICS 20 and ICS 21 implemented in this PR :point_up: + - @fedekunze, @jackzampolin and @alexanderbez to review +- Additional PR incoming for ICS 22 and ICS 23 +- Include [#382](https://github.com/cosmos/cosmos-sdk/issues/382) + +# Lower priority ## Governance v2 - Simple software upgrade proposals - Implementation described in [#1079](https://github.com/cosmos/cosmos-sdk/issues/1079) - Agree upon a block height to switch to new version - -## Staking/Slashing/Stability - -- Miscellaneous minor staking issues - - [List here](https://github.com/cosmos/cosmos-sdk/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+label%3Astaking+label%3Aprelaunch) -- Unbonding state for validators https://github.com/cosmos/cosmos-sdk/issues/1676 -- Slashing period - ref [#2001](https://github.com/cosmos/cosmos-sdk/pull/2001) (spec) - - Various other slashing issues needing resolution - ref [#1256](https://github.com/cosmos/cosmos-sdk/issues/1256) -- Update staking/slashing for NextValSet change - -## Vesting - -- Single `VestingAccount` allowing delegation/voting but no withdrawals -- Ref [#1875](https://github.com/cosmos/cosmos-sdk/pull/1875) (spec) - -## Multisig - -- Already implemented on TM side, need simple CLI interface - -## Other - -- Need to update for new ABCI changes - error string, tags are list of lists, proposer in header (Tendermint 0.24?) - -## Gas - -- Benchmark gas, choose better constants - -# Lower priority - -## Governance - -- Circuit breaker +- Another Governance proposal from @jaekwon [#2116](https://github.com/cosmos/cosmos-sdk/pull/2116) +- Circuit breaker - Parameter change proposals (roughly the same implementation as circuit breaker) ## Documentation