Skip to content

Commit

Permalink
Update x/ibc error handling (#5462)
Browse files Browse the repository at this point in the history 
* Merge PR #5428: Add mod, exponentiation for uint

* Modified examples in distribution module (#5441)

* Merge PR #5442: Remove of the client/alias.go

* Merge PR #5445: Mock rpcclient in tests for votes pagination

* Merge PR #5435: Added iterator that allows to read only requested values

* Merge PR #5427: Remove code duplication in x/auth/client/cli

* Merge PR #5421: Refactor Error Handling

* update x/ibc error handling

* update ICS24 and ICS02 errors

* ICS03, ICS23 and common errors

* updates from master and errors from ICS04

* build

* fix ics20 tests

* fix tests

* golangcibot fixes

Co-authored-by: Kevin Davis <karzak@users.noreply.github.com>
Co-authored-by: kaustubhkapatral <54210167+kaustubhkapatral@users.noreply.github.com>
Co-authored-by: Ferenc Fabian <qwer.kocka@gmail.com>
Co-authored-by: Dmitry Shulyak <yashulyak@gmail.com>
Co-authored-by: Alessio Treglia <quadrispro@ubuntu.com>
Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
  • Loading branch information
@fedekunze @karzak
@kaustubhkapatral @PumpkinSeed @dshulyak @alessio @alexanderbez
7 people authored Jan 2, 2020
1 parent e06d6a9 commit 221ce96
Show file tree
Hide file tree
Showing 265 changed files with 3,831 additions and 4,517 deletions.
26 changes: 23 additions & 3 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,21 @@ logic has been implemented for v0.38 target version. Applications can migrate vi

### API Breaking Changes

* (baseapp/types) [\#5421](https://github.com/cosmos/cosmos-sdk/pull/5421) The `Error` interface (`types/errors.go`)
has been removed in favor of the concrete type defined in `types/errors/` which implements the standard `error`
interface. As a result, the `Handler` and `Querier` implementations now return a standard `error`.
Within `BaseApp`, `runTx` now returns a `(GasInfo, *Result, error)` tuple and `runMsgs` returns a
`(*Result, error)` tuple. A reference to a `Result` is now used to indicate success whereas an error
signals an invalid message or failed message execution. As a result, the fields `Code`, `Codespace`,
`GasWanted`, and `GasUsed` have been removed the `Result` type. The latter two fields are now found
in the `GasInfo` type which is always returned regardless of execution outcome.

Note to developers: Since all handlers and queriers must now return a standard `error`, the `types/errors/`
package contains all the relevant and pre-registered errors that you typically work with. A typical
error returned will look like `sdkerrors.Wrap(sdkerrors.ErrUnknownRequest, "...")`. You can retrieve
relevant ABCI information from the error via `ABCIInfo`.
* (client) [\#5442](https://github.com/cosmos/cosmos-sdk/pull/5442) Remove client/alias.go as it's not necessary and
components can be imported directly from the packages.
* (store) [\#4748](https://github.com/cosmos/cosmos-sdk/pull/4748) The `CommitMultiStore` interface
now requires a `SetInterBlockCache` method. Applications that do not wish to support this can simply
have this method perform a no-op.
Expand All @@ -76,7 +91,7 @@ if the provided arguments are invalid.
`StdTx.Signatures` to get back the array of StdSignatures `[]StdSignature`.
* (modules) [\#5299](https://github.com/cosmos/cosmos-sdk/pull/5299) `HandleDoubleSign` along with params `MaxEvidenceAge`
and `DoubleSignJailEndTime` have moved from the `x/slashing` module to the `x/evidence` module.
* (keys) [\#4941](https://github.com/cosmos/cosmos-sdk/issues/4941) Initializing a new keybase through `NewKeyringFromHomeFlag`, `NewKeyringFromDir`, `NewKeyBaseFromHomeFlag`, `NewKeyBaseFromDir`, or `NewInMemory` functions now accept optional parameters of type `KeybaseOption`. These optional parameters are also added on the keys subcommands functions, which are now public, and allows these options to be set on the commands or ignored to default to previous behavior.
* (keys) [\#4941](https://github.com/cosmos/cosmos-sdk/issues/4941) Initializing a new keybase through `NewKeyringFromHomeFlag`, `NewKeyringFromDir`, `NewKeyBaseFromHomeFlag`, `NewKeyBaseFromDir`, or `NewInMemory` functions now accept optional parameters of type `KeybaseOption`. These optional parameters are also added on the keys subcommands functions, which are now public, and allows these options to be set on the commands or ignored to default to previous behavior.
* The option introduced in this PR is `WithKeygenFunc` which allows a custom bytes to key implementation to be defined when keys are created.
* (simapp) [\#5419](https://github.com/cosmos/cosmos-sdk/pull/5419) simapp/helpers.GenTx() now accepts a gas argument.

Expand All @@ -88,9 +103,11 @@ if the provided arguments are invalid.
increased significantly due to modular `AnteHandler` support. Increase GasLimit accordingly.
* (rest) [\#5336](https://github.com/cosmos/cosmos-sdk/issues/5336) `MsgEditValidator` uses `description` instead of `Description` as a JSON key.
* (keys) [\#5097](https://github.com/cosmos/cosmos-sdk/pull/5097) Due to the keybase -> keyring transition, keys need to be migrated. See `keys migrate` command for more info.
* (x/auth) [\#5424](https://github.com/cosmos/cosmos-sdk/issues/5424) Drop `decode-tx` command from x/auth/client/cli, duplicate of the `decode` command.

### Features

* (store) [\#5435](https://github.com/cosmos/cosmos-sdk/pull/5435) New iterator for paginated requests. Iterator limits DB reads to the range of the requested page.
* (x/evidence) [\#5240](https://github.com/cosmos/cosmos-sdk/pull/5240) Initial implementation of the `x/evidence` module.
* (cli) [\#5212](https://github.com/cosmos/cosmos-sdk/issues/5212) The `q gov proposals` command now supports pagination.
* (store) [\#4724](https://github.com/cosmos/cosmos-sdk/issues/4724) Multistore supports substore migrations upon load. New `rootmulti.Store.LoadLatestVersionAndUpgrade` method in
Expand Down Expand Up @@ -213,11 +230,12 @@ to detail this new feature and how state transitions occur.
* (docs/spec) All module specs moved into their respective module dir in x/ (i.e. docs/spec/staking -->> x/staking/spec)
* (docs/) [\#5379](https://github.com/cosmos/cosmos-sdk/pull/5379) Major documentation refactor, including:
* (docs/intro/) Add and improve introduction material for newcomers.
* (docs/basics/) Add documentation about basic concepts of the cosmos sdk such as the anatomy of an SDK application, the transaction lifecycle or accounts.
* (docs/core/) Add documentation about core conepts of the cosmos sdk such as `baseapp`, `server`, `store`s, `context` and more.
* (docs/basics/) Add documentation about basic concepts of the cosmos sdk such as the anatomy of an SDK application, the transaction lifecycle or accounts.
* (docs/core/) Add documentation about core conepts of the cosmos sdk such as `baseapp`, `server`, `store`s, `context` and more.
* (docs/building-modules/) Add reference documentation on concepts relevant for module developers (`keeper`, `handler`, `messages`, `queries`,...).
* (docs/interfaces/) Add documentation on building interfaces for the Cosmos SDK.
* Redesigned user interface that features new dynamically generated sidebar, build-time code embedding from GitHub, new homepage as well as many other improvements.
* (types) [\#5428](https://github.com/cosmos/cosmos-sdk/pull/5428) Add `Mod` (modulo) method and `RelativePow` (exponentation) function for `Uint`.
### Bug Fixes
Expand Down Expand Up @@ -2801,3 +2819,5 @@ BUG FIXES:
[v0.37.1]: https://github.com/cosmos/cosmos-sdk/releases/tag/v0.37.1
[v0.37.0]: https://github.com/cosmos/cosmos-sdk/releases/tag/v0.37.0
[v0.36.0]: https://github.com/cosmos/cosmos-sdk/releases/tag/v0.36.0
176 changes: 99 additions & 77 deletions baseapp/abci.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)

// InitChain implements the ABCI interface. It runs the initialization logic
Expand Down Expand Up @@ -153,54 +154,66 @@ func (app *BaseApp) EndBlock(req abci.RequestEndBlock) (res abci.ResponseEndBloc
return
}

// CheckTx implements the ABCI interface. It runs the "basic checks" to see
// whether or not a transaction can possibly be executed, first decoding and then
// the ante handler (which checks signatures/fees/ValidateBasic).
//
// NOTE:CheckTx does not run the actual Msg handler function(s).
func (app *BaseApp) CheckTx(req abci.RequestCheckTx) (res abci.ResponseCheckTx) {
var result sdk.Result

// CheckTx implements the ABCI interface and executes a tx in CheckTx mode. In
// CheckTx mode, messages are not executed. This means messages are only validated
// and only the AnteHandler is executed. State is persisted to the BaseApp's
// internal CheckTx state if the AnteHandler passes. Otherwise, the ResponseCheckTx
// will contain releveant error information. Regardless of tx execution outcome,
// the ResponseCheckTx will contain relevant gas execution context.
func (app *BaseApp) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx {
tx, err := app.txDecoder(req.Tx)
if err != nil {
return sdkerrors.ResponseCheckTx(err, 0, 0)
}

var mode runTxMode

switch {
case err != nil:
result = err.Result()
case req.Type == abci.CheckTxType_New:
result = app.runTx(runTxModeCheck, req.Tx, tx)
mode = runTxModeCheck

case req.Type == abci.CheckTxType_Recheck:
result = app.runTx(runTxModeReCheck, req.Tx, tx)
mode = runTxModeReCheck

default:
panic(fmt.Sprintf("Unknown RequestCheckTx Type: %v", req.Type))
panic(fmt.Sprintf("unknown RequestCheckTx type: %s", req.Type))
}

gInfo, result, err := app.runTx(mode, req.Tx, tx)
if err != nil {
return sdkerrors.ResponseCheckTx(err, gInfo.GasWanted, gInfo.GasUsed)
}

return abci.ResponseCheckTx{
Code: uint32(result.Code),
Data: result.Data,
GasWanted: int64(gInfo.GasWanted), // TODO: Should type accept unsigned ints?
GasUsed: int64(gInfo.GasUsed), // TODO: Should type accept unsigned ints?
Log: result.Log,
GasWanted: int64(result.GasWanted), // TODO: Should type accept unsigned ints?
GasUsed: int64(result.GasUsed), // TODO: Should type accept unsigned ints?
Data: result.Data,
Events: result.Events.ToABCIEvents(),
}
}

// DeliverTx implements the ABCI interface.
func (app *BaseApp) DeliverTx(req abci.RequestDeliverTx) (res abci.ResponseDeliverTx) {
var result sdk.Result

// DeliverTx implements the ABCI interface and executes a tx in DeliverTx mode.
// State only gets persisted if all messages are valid and get executed successfully.
// Otherwise, the ResponseDeliverTx will contain releveant error information.
// Regardless of tx execution outcome, the ResponseDeliverTx will contain relevant
// gas execution context.
func (app *BaseApp) DeliverTx(req abci.RequestDeliverTx) abci.ResponseDeliverTx {
tx, err := app.txDecoder(req.Tx)
if err != nil {
result = err.Result()
} else {
result = app.runTx(runTxModeDeliver, req.Tx, tx)
return sdkerrors.ResponseDeliverTx(err, 0, 0)
}

gInfo, result, err := app.runTx(runTxModeDeliver, req.Tx, tx)
if err != nil {
return sdkerrors.ResponseDeliverTx(err, gInfo.GasWanted, gInfo.GasUsed)
}

return abci.ResponseDeliverTx{
Code: uint32(result.Code),
Codespace: string(result.Codespace),
Data: result.Data,
GasWanted: int64(gInfo.GasWanted), // TODO: Should type accept unsigned ints?
GasUsed: int64(gInfo.GasUsed), // TODO: Should type accept unsigned ints?
Log: result.Log,
GasWanted: int64(result.GasWanted), // TODO: Should type accept unsigned ints?
GasUsed: int64(result.GasUsed), // TODO: Should type accept unsigned ints?
Data: result.Data,
Events: result.Events.ToABCIEvents(),
}
}
Expand Down Expand Up @@ -278,11 +291,10 @@ func (app *BaseApp) halt() {

// Query implements the ABCI interface. It delegates to CommitMultiStore if it
// implements Queryable.
func (app *BaseApp) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
func (app *BaseApp) Query(req abci.RequestQuery) abci.ResponseQuery {
path := splitPath(req.Path)
if len(path) == 0 {
msg := "no query path provided"
return sdk.ErrUnknownRequest(msg).QueryResult()
sdkerrors.QueryResult(sdkerrors.Wrap(sdkerrors.ErrUnknownRequest, "no query path provided"))
}

switch path[0] {
Expand All @@ -294,61 +306,59 @@ func (app *BaseApp) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
return handleQueryStore(app, path, req)

case "p2p":
return handleQueryP2P(app, path, req)
return handleQueryP2P(app, path)

case "custom":
return handleQueryCustom(app, path, req)
}

msg := "unknown query path"
return sdk.ErrUnknownRequest(msg).QueryResult()
return sdkerrors.QueryResult(sdkerrors.Wrap(sdkerrors.ErrUnknownRequest, "unknown query path"))
}

func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) (res abci.ResponseQuery) {
func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) abci.ResponseQuery {
if len(path) >= 2 {
var result sdk.Result

switch path[1] {
case "simulate":
txBytes := req.Data

tx, err := app.txDecoder(txBytes)
if err != nil {
result = err.Result()
} else {
result = app.Simulate(txBytes, tx)
return sdkerrors.QueryResult(sdkerrors.Wrap(err, "failed to decode tx"))
}

gInfo, _, _ := app.Simulate(txBytes, tx)

return abci.ResponseQuery{
Codespace: sdkerrors.RootCodespace,
Height: req.Height,
Value: codec.Cdc.MustMarshalBinaryLengthPrefixed(gInfo.GasUsed),
}

case "version":
return abci.ResponseQuery{
Code: uint32(sdk.CodeOK),
Codespace: string(sdk.CodespaceRoot),
Codespace: sdkerrors.RootCodespace,
Height: req.Height,
Value: []byte(app.appVersion),
}

default:
result = sdk.ErrUnknownRequest(fmt.Sprintf("unknown query: %s", path)).Result()
}

value := codec.Cdc.MustMarshalBinaryLengthPrefixed(result)
return abci.ResponseQuery{
Code: uint32(sdk.CodeOK),
Codespace: string(sdk.CodespaceRoot),
Height: req.Height,
Value: value,
return sdkerrors.QueryResult(sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "unknown query: %s", path))
}
}

msg := "expected second parameter to be either 'simulate' or 'version', neither was present"
return sdk.ErrUnknownRequest(msg).QueryResult()
return sdkerrors.QueryResult(
sdkerrors.Wrap(
sdkerrors.ErrUnknownRequest,
"expected second parameter to be either 'simulate' or 'version', neither was present",
),
)
}

func handleQueryStore(app *BaseApp, path []string, req abci.RequestQuery) abci.ResponseQuery {
// "/store" prefix for store queries
queryable, ok := app.cms.(sdk.Queryable)
if !ok {
msg := "multistore doesn't support queries"
return sdk.ErrUnknownRequest(msg).QueryResult()
return sdkerrors.QueryResult(sdkerrors.Wrap(sdkerrors.ErrUnknownRequest, "multistore doesn't support queries"))
}

req.Path = "/" + strings.Join(path[1:], "/")
Expand All @@ -359,7 +369,12 @@ func handleQueryStore(app *BaseApp, path []string, req abci.RequestQuery) abci.R
}

if req.Height <= 1 && req.Prove {
return sdk.ErrInternal("cannot query with proof when height <= 1; please provide a valid height").QueryResult()
return sdkerrors.QueryResult(
sdkerrors.Wrap(
sdkerrors.ErrInvalidRequest,
"cannot query with proof when height <= 1; please provide a valid height",
),
)
}

resp := queryable.Query(req)
Expand All @@ -368,7 +383,7 @@ func handleQueryStore(app *BaseApp, path []string, req abci.RequestQuery) abci.R
return resp
}

func handleQueryP2P(app *BaseApp, path []string, _ abci.RequestQuery) (res abci.ResponseQuery) {
func handleQueryP2P(app *BaseApp, path []string) abci.ResponseQuery {
// "/p2p" prefix for p2p queries
if len(path) >= 4 {
cmd, typ, arg := path[1], path[2], path[3]
Expand All @@ -383,28 +398,30 @@ func handleQueryP2P(app *BaseApp, path []string, _ abci.RequestQuery) (res abci.
}

default:
msg := "expected second parameter to be 'filter'"
return sdk.ErrUnknownRequest(msg).QueryResult()
return sdkerrors.QueryResult(sdkerrors.Wrap(sdkerrors.ErrUnknownRequest, "expected second parameter to be 'filter'"))
}
}

msg := "Expected path is p2p filter <addr|id> <parameter>"
return sdk.ErrUnknownRequest(msg).QueryResult()
return sdkerrors.QueryResult(
sdkerrors.Wrap(
sdkerrors.ErrUnknownRequest, "expected path is p2p filter <addr|id> <parameter>",
),
)
}

func handleQueryCustom(app *BaseApp, path []string, req abci.RequestQuery) (res abci.ResponseQuery) {
func handleQueryCustom(app *BaseApp, path []string, req abci.RequestQuery) abci.ResponseQuery {
// path[0] should be "custom" because "/custom" prefix is required for keeper
// queries.
//
// The QueryRouter routes using path[1]. For example, in the path
// "custom/gov/proposal", QueryRouter routes using "gov".
if len(path) < 2 || path[1] == "" {
return sdk.ErrUnknownRequest("No route for custom query specified").QueryResult()
return sdkerrors.QueryResult(sdkerrors.Wrap(sdkerrors.ErrUnknownRequest, "no route for custom query specified"))
}

querier := app.queryRouter.Route(path[1])
if querier == nil {
return sdk.ErrUnknownRequest(fmt.Sprintf("no custom querier found for route %s", path[1])).QueryResult()
return sdkerrors.QueryResult(sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "no custom querier found for route %s", path[1]))
}

// when a client did not provide a query height, manually inject the latest
Expand All @@ -413,17 +430,22 @@ func handleQueryCustom(app *BaseApp, path []string, req abci.RequestQuery) (res
}

if req.Height <= 1 && req.Prove {
return sdk.ErrInternal("cannot query with proof when height <= 1; please provide a valid height").QueryResult()
return sdkerrors.QueryResult(
sdkerrors.Wrap(
sdkerrors.ErrInvalidRequest,
"cannot query with proof when height <= 1; please provide a valid height",
),
)
}

cacheMS, err := app.cms.CacheMultiStoreWithVersion(req.Height)
if err != nil {
return sdk.ErrInternal(
fmt.Sprintf(
"failed to load state at height %d; %s (latest height: %d)",
req.Height, err, app.LastBlockHeight(),
return sdkerrors.QueryResult(
sdkerrors.Wrapf(
sdkerrors.ErrInvalidRequest,
"failed to load state at height %d; %s (latest height: %d)", req.Height, err, app.LastBlockHeight(),
),
).QueryResult()
)
}

// cache wrap the commit-multistore for safety
Expand All @@ -435,18 +457,18 @@ func handleQueryCustom(app *BaseApp, path []string, req abci.RequestQuery) (res
//
// For example, in the path "custom/gov/proposal/test", the gov querier gets
// []string{"proposal", "test"} as the path.
resBytes, queryErr := querier(ctx, path[2:], req)
if queryErr != nil {
resBytes, err := querier(ctx, path[2:], req)
if err != nil {
space, code, log := sdkerrors.ABCIInfo(err, false)
return abci.ResponseQuery{
Code: uint32(queryErr.Code()),
Codespace: string(queryErr.Codespace()),
Code: code,
Codespace: space,
Log: log,
Height: req.Height,
Log: queryErr.ABCILog(),
}
}

return abci.ResponseQuery{
Code: uint32(sdk.CodeOK),
Height: req.Height,
Value: resBytes,
}
Expand Down
Loading

0 comments on commit 221ce96

Please sign in to comment.