Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Better Errors #924

Merged
merged 6 commits into from
Dec 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions app/helpers/contracts.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package helpers

import (
wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types"

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

// Execute contract, recover from panic
func ExecuteContract(k wasmtypes.ContractOpsKeeper, childCtx sdk.Context, contractAddr sdk.AccAddress, msgBz []byte, err *error) {
// Recover from panic, return error
defer func() {
if recoveryError := recover(); recoveryError != nil {
// Determine error associated with panic
if isOutofGas, msg := IsOutOfGasError(recoveryError); isOutofGas {
*err = ErrOutOfGas.Wrapf("%s", msg)
} else {
*err = ErrContractExecutionPanic.Wrapf("%s", recoveryError)
}
}
}()

// Execute contract with sudo
_, *err = k.Sudo(childCtx, contractAddr, msgBz)
}

// Check if error is out of gas error
func IsOutOfGasError(err any) (bool, string) {
switch e := err.(type) {
case storetypes.ErrorOutOfGas:
return true, e.Descriptor
case storetypes.ErrorGasOverflow:
return true, e.Descriptor
default:
return false, ""
}
}
20 changes: 20 additions & 0 deletions app/helpers/global_errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package helpers

import (
errorsmod "cosmossdk.io/errors"

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

const codespace = "juno-global"

var (
ErrInvalidAddress = sdkerrors.ErrInvalidAddress
ErrContractNotRegistered = errorsmod.Register(codespace, 1, "contract not registered")
ErrContractAlreadyRegistered = errorsmod.Register(codespace, 2, "contract already registered")
ErrContractNotAdmin = errorsmod.Register(codespace, 3, "sender is not the contract admin")
ErrContractNotCreator = errorsmod.Register(codespace, 4, "sender is not the contract creator")
ErrInvalidCWContract = errorsmod.Register(codespace, 5, "invalid CosmWasm contract")
ErrOutOfGas = errorsmod.Register(codespace, 6, "contract execution ran out of gas")
ErrContractExecutionPanic = errorsmod.Register(codespace, 7, "contract execution panicked")
)
4 changes: 4 additions & 0 deletions interchaintest/contracts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ A list of the contracts here which are pre-compiled in other repos.
> ibchooks_counter.wasm -> <https://github.com/osmosis-labs/osmosis/blob/64393a14e18b2562d72a3892eec716197a3716c7/tests/ibc-hooks/bytecode/counter.wasm>
> cw_testburn.wasm -> <https://github.com/Reecepbcups/cw-testburn>
> clock_example.wasm -> <https://github.com/Reecepbcups/cw-clock-example>
> clock_example_high_gas.wasm -> <https://github.com/joelsmith-2019/cw-clock-example/releases/tag/v1.0-high-gas>
> clock_example_migrate.wasm -> <https://github.com/joelsmith-2019/cw-clock-example/releases/tag/v1.0-migrate>
> clock_example_no_sudo.wasm -> <https://github.com/joelsmith-2019/cw-clock-example/releases/tag/v1.0-no-sudo>
> juno_staking_hooks_example.wasm -> <https://github.com/Reecepbcups/cw-juno-staking-hooks-example>
> juno_staking_hooks_high_gas_example.wasm -> <https://github.com/joelsmith-2019/juno-cwhooks-example/releases/tag/v1.0-high-gas>

> cw721_base - https://github.com/CosmWasm/cw-nfts/releases/download/v0.17.0/cw721_base.wasm
> cw721-receiver - https://github.com/CosmWasm/cw-nfts/pull/144
Binary file not shown.
19 changes: 19 additions & 0 deletions interchaintest/module_cwhooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,25 @@ func TestJunoCwHooks(t *testing.T) {
require.Equal(t, user.FormattedAddress(), res.Data.DelegatorAddress)
require.Equal(t, fmt.Sprintf("%d.000000000000000000", stakeAmt), res.Data.Shares)

// HIGH GAS TEST
// Setup a high gas contract
_, contractAddr = helpers.SetupContract(t, ctx, juno, user.KeyName(), "contracts/juno_staking_hooks_high_gas_example.wasm", `{}`)
joelsmith-2019 marked this conversation as resolved.
Show resolved Hide resolved

// Register staking contract
helpers.RegisterCwHooksStaking(t, ctx, juno, user, contractAddr)
sc = helpers.GetCwHooksStakingContracts(t, ctx, juno)
require.Equal(t, 2, len(sc))

// Perform a Staking Action
stakeAmt = 1_000_000
helpers.StakeTokens(t, ctx, juno, user, valoper, fmt.Sprintf("%d%s", stakeAmt, juno.Config().Denom))

// Query the smart contract, should panic and not update value
res = helpers.GetCwStakingHookLastDelegationChange(t, ctx, juno, contractAddr, user.FormattedAddress())
require.Equal(t, "", res.Data.ValidatorAddress)
require.Equal(t, "", res.Data.DelegatorAddress)
require.Equal(t, "", res.Data.Shares)

t.Cleanup(func() {
_ = ic.Close()
})
Expand Down
34 changes: 2 additions & 32 deletions x/clock/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import (

"github.com/cometbft/cometbft/libs/log"

storetypes "github.com/cosmos/cosmos-sdk/store/types"
"github.com/cosmos/cosmos-sdk/telemetry"
sdk "github.com/cosmos/cosmos-sdk/types"

helpers "github.com/CosmosContracts/juno/v19/app/helpers"
"github.com/CosmosContracts/juno/v19/x/clock/keeper"
"github.com/CosmosContracts/juno/v19/x/clock/types"
)
Expand Down Expand Up @@ -51,7 +51,7 @@ func EndBlocker(ctx sdk.Context, k keeper.Keeper) {
childCtx := ctx.WithGasMeter(sdk.NewGasMeter(p.ContractGasLimit))

// Execute contract
executeContract(k, childCtx, contractAddr, &err)
helpers.ExecuteContract(k.GetContractKeeper(), childCtx, contractAddr, endBlockSudoMessage, &err)
if handleError(ctx, k, logger, errorExecs, &errorExists, err, idx, contract.ContractAddress) {
continue
}
Expand Down Expand Up @@ -90,33 +90,3 @@ func handleError(

return err != nil
}

// Execute contract, recover from panic
func executeContract(k keeper.Keeper, childCtx sdk.Context, contractAddr sdk.AccAddress, err *error) {
// Recover from panic, return error
defer func() {
if recoveryError := recover(); recoveryError != nil {
// Determine error associated with panic
if isOutofGas, msg := isOutOfGasError(recoveryError); isOutofGas {
*err = types.ErrOutOfGas.Wrapf("%s", msg)
} else {
*err = types.ErrContractExecutionPanic.Wrapf("%s", recoveryError)
}
}
}()

// Execute contract with sudo
_, *err = k.GetContractKeeper().Sudo(childCtx, contractAddr, endBlockSudoMessage)
}

// Check if error is out of gas error
func isOutOfGasError(err any) (bool, string) {
switch e := err.(type) {
case storetypes.ErrorOutOfGas:
return true, e.Descriptor
case storetypes.ErrorGasOverflow:
return true, e.Descriptor
default:
return false, ""
}
}
13 changes: 7 additions & 6 deletions x/clock/keeper/clock.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/query"

globalerrors "github.com/CosmosContracts/juno/v19/app/helpers"
"github.com/CosmosContracts/juno/v19/x/clock/types"
)

Expand Down Expand Up @@ -42,7 +43,7 @@ func (k Keeper) IsClockContract(ctx sdk.Context, contractAddress string) bool {
func (k Keeper) GetClockContract(ctx sdk.Context, contractAddress string) (*types.ClockContract, error) {
// Check if the contract is registered
if !k.IsClockContract(ctx, contractAddress) {
return nil, types.ErrContractNotRegistered
return nil, globalerrors.ErrContractNotRegistered
}

// Get the KV store
Expand Down Expand Up @@ -134,7 +135,7 @@ func (k Keeper) RemoveContract(ctx sdk.Context, contractAddress string) {
func (k Keeper) RegisterContract(ctx sdk.Context, senderAddress string, contractAddress string) error {
// Check if the contract is already registered
if k.IsClockContract(ctx, contractAddress) {
return types.ErrContractAlreadyRegistered
return globalerrors.ErrContractAlreadyRegistered
}

// Ensure the sender is the contract admin or creator
Expand All @@ -153,7 +154,7 @@ func (k Keeper) RegisterContract(ctx sdk.Context, senderAddress string, contract
func (k Keeper) UnregisterContract(ctx sdk.Context, senderAddress string, contractAddress string) error {
// Check if the contract is registered in either store
if !k.IsClockContract(ctx, contractAddress) {
return types.ErrContractNotRegistered
return globalerrors.ErrContractNotRegistered
}

// Ensure the sender is the contract admin or creator
Expand Down Expand Up @@ -208,7 +209,7 @@ func (k Keeper) IsContractManager(ctx sdk.Context, senderAddress string, contrac

// Ensure the contract is a cosm wasm contract
if ok := k.wasmKeeper.HasContractInfo(ctx, contractAddr); !ok {
return false, types.ErrInvalidCWContract
return false, globalerrors.ErrInvalidCWContract
}

// Get the contract info
Expand All @@ -221,9 +222,9 @@ func (k Keeper) IsContractManager(ctx sdk.Context, senderAddress string, contrac

// Check if the sender is the admin or creator
if adminExists && !isSenderAdmin {
return false, types.ErrContractNotAdmin
return false, globalerrors.ErrContractNotAdmin
} else if !adminExists && !isSenderCreator {
return false, types.ErrContractNotCreator
return false, globalerrors.ErrContractNotCreator
}

return true, nil
Expand Down
3 changes: 2 additions & 1 deletion x/clock/keeper/querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

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

globalerrors "github.com/CosmosContracts/juno/v19/app/helpers"
"github.com/CosmosContracts/juno/v19/x/clock/types"
)

Expand Down Expand Up @@ -38,7 +39,7 @@ func (q Querier) ClockContract(stdCtx context.Context, req *types.QueryClockCont

// Ensure the contract address is valid
if _, err := sdk.AccAddressFromBech32(req.ContractAddress); err != nil {
return nil, types.ErrInvalidAddress
return nil, globalerrors.ErrInvalidAddress
}

contract, err := q.keeper.GetClockContract(ctx, req.ContractAddress)
Expand Down
17 changes: 3 additions & 14 deletions x/clock/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,10 @@ package types

import (
errorsmod "cosmossdk.io/errors"

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

var (
ErrInvalidAddress = sdkerrors.ErrInvalidAddress
ErrContractNotRegistered = errorsmod.Register(ModuleName, 1, "contract not registered")
ErrContractAlreadyRegistered = errorsmod.Register(ModuleName, 2, "contract already registered")
ErrContractRegisterNotAdmin = errorsmod.Register(ModuleName, 3, "this address is not the contract admin, cannot register")
ErrContractNotAdmin = errorsmod.Register(ModuleName, 4, "sender is not the contract admin")
ErrContractNotCreator = errorsmod.Register(ModuleName, 5, "sender is not the contract creator")
ErrContractJailed = errorsmod.Register(ModuleName, 6, "contract is jailed")
ErrContractNotJailed = errorsmod.Register(ModuleName, 7, "contract is not jailed")
ErrContractAlreadyJailed = errorsmod.Register(ModuleName, 8, "contract is already jailed")
ErrInvalidCWContract = errorsmod.Register(ModuleName, 9, "invalid CosmWasm contract")
ErrOutOfGas = errorsmod.Register(ModuleName, 10, "contract execution ran out of gas")
ErrContractExecutionPanic = errorsmod.Register(ModuleName, 11, "contract execution panicked")
ErrContractJailed = errorsmod.Register(ModuleName, 1, "contract is jailed")
ErrContractNotJailed = errorsmod.Register(ModuleName, 2, "contract is not jailed")
ErrContractAlreadyJailed = errorsmod.Register(ModuleName, 3, "contract is already jailed")
)
4 changes: 3 additions & 1 deletion x/clock/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"cosmossdk.io/errors"

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

globalerrors "github.com/CosmosContracts/juno/v19/app/helpers"
)

const (
Expand Down Expand Up @@ -133,7 +135,7 @@ func (msg *MsgUpdateParams) ValidateBasic() error {
func validateAddresses(addresses ...string) error {
for _, address := range addresses {
if _, err := sdk.AccAddressFromBech32(address); err != nil {
return errors.Wrapf(ErrInvalidAddress, "invalid address: %s", address)
return errors.Wrapf(globalerrors.ErrInvalidAddress, "invalid address: %s", address)
}
}

Expand Down
7 changes: 6 additions & 1 deletion x/cw-hooks/keeper/contracts.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package keeper
import (
"github.com/cosmos/cosmos-sdk/store/prefix"
sdk "github.com/cosmos/cosmos-sdk/types"

helpers "github.com/CosmosContracts/juno/v19/app/helpers"
)

func (k Keeper) SetContract(ctx sdk.Context, keyPrefix []byte, contractAddr sdk.AccAddress) {
Expand Down Expand Up @@ -63,7 +65,10 @@ func (k Keeper) ExecuteMessageOnContracts(ctx sdk.Context, keyPrefix []byte, msg
for _, c := range k.GetAllContracts(ctx, keyPrefix) {
gasLimitCtx := ctx.WithGasMeter(sdk.NewGasMeter(p.ContractGasLimit))
addr := sdk.AccAddress(c.Bytes())
if _, err := k.GetContractKeeper().Sudo(gasLimitCtx, addr, msgBz); err != nil {

var err error
helpers.ExecuteContract(k.GetContractKeeper(), gasLimitCtx, addr, msgBz, &err)
if err != nil {
k.Logger(ctx).Error("ExecuteMessageOnContracts err", err, "contract", addr.String())
return err
}
Expand Down
15 changes: 8 additions & 7 deletions x/feepay/keeper/feepay.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/query"

globalerrors "github.com/CosmosContracts/juno/v19/app/helpers"
"github.com/CosmosContracts/juno/v19/x/feepay/types"
)

Expand All @@ -22,7 +23,7 @@ func (k Keeper) IsContractRegistered(ctx sdk.Context, contractAddr string) bool
func (k Keeper) GetContract(ctx sdk.Context, contractAddress string) (*types.FeePayContract, error) {
// Return nil, contract not registered
if !k.IsContractRegistered(ctx, contractAddress) {
return nil, types.ErrContractNotRegistered
return nil, globalerrors.ErrContractNotRegistered
}

store := prefix.NewStore(ctx.KVStore(k.storeKey), StoreKeyContracts)
Expand Down Expand Up @@ -92,7 +93,7 @@ func (k Keeper) GetAllContracts(ctx sdk.Context) []types.FeePayContract {
func (k Keeper) RegisterContract(ctx sdk.Context, rfp *types.MsgRegisterFeePayContract) error {
// Return false because the contract was already registered
if k.IsContractRegistered(ctx, rfp.FeePayContract.ContractAddress) {
return types.ErrContractAlreadyRegistered
return globalerrors.ErrContractAlreadyRegistered
}

// Check if sender is the owner of the cw contract
Expand All @@ -102,7 +103,7 @@ func (k Keeper) RegisterContract(ctx sdk.Context, rfp *types.MsgRegisterFeePayCo
}

if ok := k.wasmKeeper.HasContractInfo(ctx, contractAddr); !ok {
return types.ErrInvalidCWContract
return globalerrors.ErrInvalidCWContract
}

// Get the contract owner
Expand Down Expand Up @@ -141,7 +142,7 @@ func (k Keeper) UnregisterContract(ctx sdk.Context, rfp *types.MsgUnregisterFeeP

// Ensure CW contract is valid
if ok := k.wasmKeeper.HasContractInfo(ctx, contractAddr); !ok {
return types.ErrInvalidCWContract
return globalerrors.ErrInvalidCWContract
}

// Get the contract info
Expand Down Expand Up @@ -278,7 +279,7 @@ func (k Keeper) UpdateContractWalletLimit(ctx sdk.Context, fpc *types.FeePayCont
}

if ok := k.wasmKeeper.HasContractInfo(ctx, contractAddr); !ok {
return types.ErrInvalidCWContract
return globalerrors.ErrInvalidCWContract
}

// Get the contract info & ensure sender is the manager
Expand Down Expand Up @@ -316,9 +317,9 @@ func (k Keeper) IsContractManager(senderAddress string, contractInfo *wasmtypes.
isSenderCreator := contractInfo.Creator == senderAddress

if adminExists && !isSenderAdmin {
return false, types.ErrContractNotAdmin
return false, globalerrors.ErrContractNotAdmin
} else if !adminExists && !isSenderCreator {
return false, types.ErrContractNotCreator
return false, globalerrors.ErrContractNotCreator
}

return true, nil
Expand Down
3 changes: 2 additions & 1 deletion x/feepay/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"

globalerrors "github.com/CosmosContracts/juno/v19/app/helpers"
"github.com/CosmosContracts/juno/v19/x/feepay/types"
)

Expand Down Expand Up @@ -39,7 +40,7 @@ func (k Keeper) FundFeePayContract(goCtx context.Context, msg *types.MsgFundFeeP
// Validate sender address
senderAddr, err := sdk.AccAddressFromBech32(msg.SenderAddress)
if err != nil {
return nil, errorsmod.Wrapf(types.ErrInvalidAddress, "invalid sender address: %s", msg.SenderAddress)
return nil, errorsmod.Wrapf(globalerrors.ErrInvalidAddress, "invalid sender address: %s", msg.SenderAddress)
}

return &types.MsgFundFeePayContractResponse{}, k.FundContract(ctx, contract, senderAddr, msg.Amount)
Expand Down
Loading
Loading