Skip to content

Commit

Permalink
fix!: queries that panic should return ack-err (#86)
Browse files Browse the repository at this point in the history
* adding ApplyFuncIfNoError

* Moving cache_ctx.go to the keeper

* updating comment

* chore: address linter errors

---------

Co-authored-by: jtieri <justin@thetieris.com>
(cherry picked from commit 83f866e)

# Conflicts:
#	modules/async-icq/testing/demo-simapp/app/export.go
  • Loading branch information
VitalyV1337 authored and mergify[bot] committed Nov 7, 2023
1 parent 5d1687d commit d2e2016
Show file tree
Hide file tree
Showing 4 changed files with 279 additions and 3 deletions.
9 changes: 7 additions & 2 deletions modules/async-icq/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -178,15 +178,20 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet) ([]byt

if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
// UnmarshalJSON errors are indeterminate and therefore are not wrapped and included in failed acks
return nil, sdkerrors.Wrapf(types.ErrUnknownDataType, "cannot unmarshal ICQ packet data")
return nil, errors.Wrapf(types.ErrUnknownDataType, "cannot unmarshal ICQ packet data")
}

reqs, err := types.DeserializeCosmosQuery(data.GetData())
if err != nil {
return nil, err
}

response, err := k.executeQuery(ctx, reqs)
// If we panic when executing a query it should be returned as an error.
var response []byte
err = applyFuncIfNoError(ctx, func(ctx sdk.Context) error {
response, err = k.executeQuery(ctx, reqs)
return err
})
if err != nil {
return nil, err
}
Expand Down
80 changes: 80 additions & 0 deletions modules/async-icq/keeper/cache_ctx.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package keeper

// This file was copied from here: https://github.com/osmosis-labs/osmosis/blob/62757d309957fa9e02e6fb0b5dc8caf1ca68e696/osmoutils/cache_ctx.go

import (
"errors"
"runtime"
"runtime/debug"

"github.com/cosmos/cosmos-sdk/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"
)

// This function lets you run the function f, but if theres an error or panic
// drop the state machine change and log the error.
// If there is no error, proceeds as normal (but with some slowdown due to SDK store weirdness)
// Try to avoid usage of iterators in f.
//
// If its an out of gas panic, this function will also panic like in normal tx execution flow.
// This is still safe for beginblock / endblock code though, as they do not have out of gas panics.
func applyFuncIfNoError(ctx sdk.Context, f func(ctx sdk.Context) error) (err error) {
// Add a panic safeguard
defer func() {
if recoveryError := recover(); recoveryError != nil {
if isErr, _ := isOutOfGasError(recoveryError); isErr {
// We panic with the same error, to replicate the normal tx execution flow.
panic(recoveryError)
} else {
printPanicRecoveryError(ctx, recoveryError)
err = errors.New("panic occurred during execution")
}
}
}()
// makes a new cache context, which all state changes get wrapped inside of.
cacheCtx, write := ctx.CacheContext()
err = f(cacheCtx)
if err != nil {
ctx.Logger().Error(err.Error())
} else {
// no error, write the output of f
write()
ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events())
}
return err
}

// Frustratingly, this has to return the error descriptor, not an actual error itself
// because the SDK errors here are not actually errors. (They don't implement error interface)
func isOutOfGasError(err any) (bool, string) {
switch e := err.(type) {
case types.ErrorOutOfGas:
return true, e.Descriptor
case types.ErrorGasOverflow:
return true, e.Descriptor
default:
return false, ""
}
}

// printPanicRecoveryError error logs the recoveryError, along with the stacktrace, if it can be parsed.
// If not emits them to stdout.
func printPanicRecoveryError(ctx sdk.Context, recoveryError interface{}) {
errStackTrace := string(debug.Stack())
switch e := recoveryError.(type) {
case types.ErrorOutOfGas:
ctx.Logger().Debug("out of gas error inside panic recovery block: " + e.Descriptor)
return
case string:
ctx.Logger().Error("Recovering from (string) panic: " + e)
case runtime.Error:
ctx.Logger().Error("recovered (runtime.Error) panic: " + e.Error())
case error:
ctx.Logger().Error("recovered (error) panic: " + e.Error())
default:
ctx.Logger().Error("recovered (default) panic. Could not capture logs in ctx, see stdout")
debug.PrintStack()
return
}
ctx.Logger().Error("stack trace: " + errStackTrace)
}
7 changes: 6 additions & 1 deletion modules/async-icq/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet) ([]byt
return nil, err
}

response, err := k.executeQuery(ctx, reqs)
// If we panic when executing a query it should be returned as an error.
var response []byte
err = applyFuncIfNoError(ctx, func(ctx sdk.Context) error {
response, err = k.executeQuery(ctx, reqs)
return err
})
if err != nil {
return nil, err
}
Expand Down
186 changes: 186 additions & 0 deletions modules/async-icq/testing/demo-simapp/app/export.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
package app

Check failure on line 1 in modules/async-icq/testing/demo-simapp/app/export.go

View workflow job for this annotation

GitHub Actions / Linter (./modules/async-icq)

: # github.com/cosmos/ibc-apps/modules/async-icq/v6/testing/demo-simapp/app

import (
"encoding/json"
"log"

tmproto "github.com/tendermint/tendermint/proto/tendermint/types"

servertypes "github.com/cosmos/cosmos-sdk/server/types"
sdk "github.com/cosmos/cosmos-sdk/types"
slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types"
"github.com/cosmos/cosmos-sdk/x/staking"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
)

// ExportAppStateAndValidators exports the state of the application for a genesis
// file.
func (app *App) ExportAppStateAndValidators(
forZeroHeight bool, jailAllowedAddrs []string,
) (servertypes.ExportedApp, error) {

// as if they could withdraw from the start of the next block
ctx := app.NewContext(true, tmproto.Header{Height: app.LastBlockHeight()})

// We export at last height + 1, because that's the height at which
// Tendermint will start InitChain.
height := app.LastBlockHeight() + 1
if forZeroHeight {
height = 0
app.prepForZeroHeightGenesis(ctx, jailAllowedAddrs)
}

genState := app.mm.ExportGenesis(ctx, app.appCodec)
appState, err := json.MarshalIndent(genState, "", " ")
if err != nil {
return servertypes.ExportedApp{}, err
}

validators, err := staking.WriteValidators(ctx, app.StakingKeeper)
if err != nil {
return servertypes.ExportedApp{}, err
}
return servertypes.ExportedApp{
AppState: appState,
Validators: validators,
Height: height,
ConsensusParams: app.BaseApp.GetConsensusParams(ctx),
}, nil
}

// prepare for fresh start at zero height
// NOTE zero height genesis is a temporary feature which will be deprecated
//
// in favour of export at a block height
func (app *App) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []string) {
applyAllowedAddrs := false

// check if there is a allowed address list
if len(jailAllowedAddrs) > 0 {
applyAllowedAddrs = true
}

allowedAddrsMap := make(map[string]bool)

for _, addr := range jailAllowedAddrs {
_, err := sdk.ValAddressFromBech32(addr)
if err != nil {
log.Fatal(err)
}
allowedAddrsMap[addr] = true
}

/* Just to be safe, assert the invariants on current state. */
app.CrisisKeeper.AssertInvariants(ctx)

/* Handle fee distribution state. */

// withdraw all validator commission
app.StakingKeeper.IterateValidators(ctx, func(_ int64, val stakingtypes.ValidatorI) (stop bool) {
_, err := app.DistrKeeper.WithdrawValidatorCommission(ctx, val.GetOperator())
if err != nil {
panic(err)
}
return false
})

// withdraw all delegator rewards
dels := app.StakingKeeper.GetAllDelegations(ctx)
for _, delegation := range dels {
_, err := app.DistrKeeper.WithdrawDelegationRewards(ctx, delegation.GetDelegatorAddr(), delegation.GetValidatorAddr())
if err != nil {
panic(err)
}
}

// clear validator slash events
app.DistrKeeper.DeleteAllValidatorSlashEvents(ctx)

// clear validator historical rewards
app.DistrKeeper.DeleteAllValidatorHistoricalRewards(ctx)

// set context height to zero
height := ctx.BlockHeight()
ctx = ctx.WithBlockHeight(0)

// reinitialize all validators
app.StakingKeeper.IterateValidators(ctx, func(_ int64, val stakingtypes.ValidatorI) (stop bool) {
// donate any unwithdrawn outstanding reward fraction tokens to the community pool
scraps := app.DistrKeeper.GetValidatorOutstandingRewardsCoins(ctx, val.GetOperator())
feePool := app.DistrKeeper.GetFeePool(ctx)
feePool.CommunityPool = feePool.CommunityPool.Add(scraps...)
app.DistrKeeper.SetFeePool(ctx, feePool)

app.DistrKeeper.Hooks().AfterValidatorCreated(ctx, val.GetOperator())
return false
})

// reinitialize all delegations
for _, del := range dels {
app.DistrKeeper.Hooks().BeforeDelegationCreated(ctx, del.GetDelegatorAddr(), del.GetValidatorAddr())
app.DistrKeeper.Hooks().AfterDelegationModified(ctx, del.GetDelegatorAddr(), del.GetValidatorAddr())
}

// reset context height
ctx = ctx.WithBlockHeight(height)

/* Handle staking state. */

// iterate through redelegations, reset creation height
app.StakingKeeper.IterateRedelegations(ctx, func(_ int64, red stakingtypes.Redelegation) (stop bool) {
for i := range red.Entries {
red.Entries[i].CreationHeight = 0
}
app.StakingKeeper.SetRedelegation(ctx, red)
return false
})

// iterate through unbonding delegations, reset creation height
app.StakingKeeper.IterateUnbondingDelegations(ctx, func(_ int64, ubd stakingtypes.UnbondingDelegation) (stop bool) {
for i := range ubd.Entries {
ubd.Entries[i].CreationHeight = 0
}
app.StakingKeeper.SetUnbondingDelegation(ctx, ubd)
return false
})

// Iterate through validators by power descending, reset bond heights, and
// update bond intra-tx counters.
store := ctx.KVStore(app.keys[stakingtypes.StoreKey])
iter := sdk.KVStoreReversePrefixIterator(store, stakingtypes.ValidatorsKey)
counter := int16(0)

for ; iter.Valid(); iter.Next() {
addr := sdk.ValAddress(iter.Key()[1:])
validator, found := app.StakingKeeper.GetValidator(ctx, addr)
if !found {
panic("expected validator, not found")
}

validator.UnbondingHeight = 0
if applyAllowedAddrs && !allowedAddrsMap[addr.String()] {
validator.Jailed = true
}

app.StakingKeeper.SetValidator(ctx, validator)
counter++
}

iter.Close()

if _, err := app.StakingKeeper.ApplyAndReturnValidatorSetUpdates(ctx); err != nil {
panic(err)
}

/* Handle slashing state. */

// reset start height on signing infos
app.SlashingKeeper.IterateValidatorSigningInfos(
ctx,
func(addr sdk.ConsAddress, info slashingtypes.ValidatorSigningInfo) (stop bool) {
info.StartHeight = 0
app.SlashingKeeper.SetValidatorSigningInfo(ctx, addr, info)
return false
},
)
}

0 comments on commit d2e2016

Please sign in to comment.