Skip to content

Commit

Permalink
Cleanup ErrNotFound cases (backport #1334) (#1343)
Browse files Browse the repository at this point in the history 
* Cleanup ErrNotFound cases

(cherry picked from commit 127d9fd)

# Conflicts:
#	x/wasm/keeper/keeper.go
#	x/wasm/keeper/querier_test.go
#	x/wasm/module_test.go

* fix conflicts

---------

Co-authored-by: Pino' Surace <[email protected]>
  • Loading branch information
@mergify @pinosu
mergify[bot] and pinosu authored Apr 20, 2023
1 parent 9e65a54 commit d2b504a
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 31 deletions.
24 changes: 14 additions & 10 deletions x/wasm/keeper/keeper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func (k Keeper) instantiate(

codeInfo := k.GetCodeInfo(ctx, codeID)
if codeInfo == nil {
return nil, nil, sdkerrors.Wrap(types.ErrNotFound, "code")
return nil, nil, types.ErrNoSuchCodeFn(codeID).Wrapf("code id %d", codeID)
}
if !authPolicy.CanInstantiateContract(codeInfo.InstantiateConfig, creator) {
return nil, nil, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "can not instantiate")
Expand Down Expand Up @@ -711,14 +711,16 @@ func (k Keeper) contractInstance(ctx sdk.Context, contractAddress sdk.AccAddress

contractBz := store.Get(types.GetContractAddressKey(contractAddress))
if contractBz == nil {
return types.ContractInfo{}, types.CodeInfo{}, nil, sdkerrors.Wrap(types.ErrNotFound, "contract")
return types.ContractInfo{}, types.CodeInfo{}, nil, types.ErrNoSuchContractFn(contractAddress.String()).
Wrapf("address %s", contractAddress.String())
}
var contractInfo types.ContractInfo
k.cdc.MustUnmarshal(contractBz, &contractInfo)

codeInfoBz := store.Get(types.GetCodeKey(contractInfo.CodeID))
if codeInfoBz == nil {
return contractInfo, types.CodeInfo{}, nil, sdkerrors.Wrap(types.ErrNotFound, "code info")
return contractInfo, types.CodeInfo{}, nil, types.ErrNoSuchCodeFn(contractInfo.CodeID).
Wrapf("code id %d", contractInfo.CodeID)
}
var codeInfo types.CodeInfo
k.cdc.MustUnmarshal(codeInfoBz, &codeInfo)
Expand Down Expand Up @@ -840,7 +842,7 @@ func (k Keeper) GetByteCode(ctx sdk.Context, codeID uint64) ([]byte, error) {
func (k Keeper) pinCode(ctx sdk.Context, codeID uint64) error {
codeInfo := k.GetCodeInfo(ctx, codeID)
if codeInfo == nil {
return sdkerrors.Wrap(types.ErrNotFound, "code info")
return types.ErrNoSuchCodeFn(codeID).Wrapf("code id %d", codeID)
}

if err := k.wasmVM.Pin(codeInfo.CodeHash); err != nil {
Expand All @@ -861,7 +863,7 @@ func (k Keeper) pinCode(ctx sdk.Context, codeID uint64) error {
func (k Keeper) unpinCode(ctx sdk.Context, codeID uint64) error {
codeInfo := k.GetCodeInfo(ctx, codeID)
if codeInfo == nil {
return sdkerrors.Wrap(types.ErrNotFound, "code info")
return types.ErrNoSuchCodeFn(codeID).Wrapf("code id %d", codeID)
}
if err := k.wasmVM.Unpin(codeInfo.CodeHash); err != nil {
return sdkerrors.Wrap(types.ErrUnpinContractFailed, err.Error())
Expand Down Expand Up @@ -890,9 +892,10 @@ func (k Keeper) InitializePinnedCodes(ctx sdk.Context) error {
defer iter.Close()

for ; iter.Valid(); iter.Next() {
codeInfo := k.GetCodeInfo(ctx, types.ParsePinnedCodeIndex(iter.Key()))
codeID := types.ParsePinnedCodeIndex(iter.Key())
codeInfo := k.GetCodeInfo(ctx, codeID)
if codeInfo == nil {
return sdkerrors.Wrap(types.ErrNotFound, "code info")
return types.ErrNoSuchCodeFn(codeID).Wrapf("code id %d", codeID)
}
if err := k.wasmVM.Pin(codeInfo.CodeHash); err != nil {
return sdkerrors.Wrap(types.ErrPinContractFailed, err.Error())
Expand All @@ -905,7 +908,8 @@ func (k Keeper) InitializePinnedCodes(ctx sdk.Context) error {
func (k Keeper) setContractInfoExtension(ctx sdk.Context, contractAddr sdk.AccAddress, ext types.ContractInfoExtension) error {
info := k.GetContractInfo(ctx, contractAddr)
if info == nil {
return sdkerrors.Wrap(types.ErrNotFound, "contract info")
return types.ErrNoSuchContractFn(contractAddr.String()).
Wrapf("address %s", contractAddr.String())
}
if err := info.SetExtension(ext); err != nil {
return err
Expand All @@ -918,7 +922,7 @@ func (k Keeper) setContractInfoExtension(ctx sdk.Context, contractAddr sdk.AccAd
func (k Keeper) setAccessConfig(ctx sdk.Context, codeID uint64, caller sdk.AccAddress, newConfig types.AccessConfig, authz AuthorizationPolicy) error {
info := k.GetCodeInfo(ctx, codeID)
if info == nil {
return sdkerrors.Wrap(types.ErrNotFound, "code info")
return types.ErrNoSuchCodeFn(codeID).Wrapf("code id %d", codeID)
}
isSubset := newConfig.Permission.IsSubset(k.getInstantiateAccessConfig(ctx))
if !authz.CanModifyCodeAccessConfig(sdk.MustAccAddressFromBech32(info.Creator), caller, isSubset) {
Expand Down Expand Up @@ -1025,7 +1029,7 @@ func (k Keeper) importAutoIncrementID(ctx sdk.Context, lastIDKey []byte, val uin

func (k Keeper) importContract(ctx sdk.Context, contractAddr sdk.AccAddress, c *types.ContractInfo, state []types.Model, entries []types.ContractCodeHistoryEntry) error {
if !k.containsCodeInfo(ctx, c.CodeID) {
return sdkerrors.Wrapf(types.ErrNotFound, "code id: %d", c.CodeID)
return types.ErrNoSuchCodeFn(c.CodeID).Wrapf("code id %d", c.CodeID)
}
if k.HasContractInfo(ctx, contractAddr) {
return sdkerrors.Wrapf(types.ErrDuplicate, "contract: %s", contractAddr)
Expand Down
4 changes: 2 additions & 2 deletions x/wasm/keeper/keeper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,7 @@ func TestInstantiateWithNonExistingCodeID(t *testing.T) {

const nonExistingCodeID = 9999
addr, _, err := keepers.ContractKeeper.Instantiate(ctx, nonExistingCodeID, creator, nil, initMsgBz, "demo contract 2", nil)
require.True(t, types.ErrNotFound.Is(err), err)
require.Equal(t, types.ErrNoSuchCodeFn(nonExistingCodeID).Wrapf("code id %d", nonExistingCodeID).Error(), err.Error())
require.Nil(t, addr)
}

Expand Down Expand Up @@ -979,7 +979,7 @@ func TestExecuteWithNonExistingAddress(t *testing.T) {
// unauthorized - trialCtx so we don't change state
nonExistingAddress := RandomAccountAddress(t)
_, err := keeper.Execute(ctx, nonExistingAddress, creator, []byte(`{}`), nil)
require.True(t, types.ErrNotFound.Is(err), err)
require.Equal(t, types.ErrNoSuchContractFn(nonExistingAddress.String()).Wrapf("address %s", nonExistingAddress.String()).Error(), err.Error())
}

func TestExecuteWithPanic(t *testing.T) {
Expand Down
5 changes: 2 additions & 3 deletions x/wasm/keeper/legacy_querier_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,15 @@ func TestLegacyQueryContractState(t *testing.T) {
},
"query smart with unknown address": {
srcPath: []string{QueryGetContractState, anyAddr.String(), QueryMethodContractStateSmart},
srcReq: abci.RequestQuery{Data: []byte(`{}`)},
srcReq: abci.RequestQuery{Data: []byte(`{"verifier":{}}`)},
expModelLen: 0,
expErr: types.ErrNotFound,
expErr: types.ErrNoSuchContractFn(anyAddr.String()).Wrapf("address %s", anyAddr.String()),
},
}

for msg, spec := range specs {
t.Run(msg, func(t *testing.T) {
binResult, err := q(ctx, spec.srcPath, spec.srcReq)
// require.True(t, spec.expErr.Is(err), "unexpected error")
require.True(t, errors.Is(err, spec.expErr), err)

// if smart query, check custom response
Expand Down
17 changes: 11 additions & 6 deletions x/wasm/keeper/querier.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ func (q grpcQuerier) ContractInfo(c context.Context, req *types.QueryContractInf
case err != nil:
return nil, err
case rsp == nil:
return nil, types.ErrNotFound
return nil, types.ErrNoSuchContractFn(contractAddr.String()).
Wrapf("address %s", contractAddr.String())
}
return rsp, nil
}
Expand Down Expand Up @@ -119,7 +120,8 @@ func (q grpcQuerier) AllContractState(c context.Context, req *types.QueryAllCont
}
ctx := sdk.UnwrapSDKContext(c)
if !q.keeper.HasContractInfo(ctx, contractAddr) {
return nil, types.ErrNotFound
return nil, types.ErrNoSuchContractFn(contractAddr.String()).
Wrapf("address %s", contractAddr.String())
}

r := make([]types.Model, 0)
Expand Down Expand Up @@ -154,7 +156,8 @@ func (q grpcQuerier) RawContractState(c context.Context, req *types.QueryRawCont
}

if !q.keeper.HasContractInfo(ctx, contractAddr) {
return nil, types.ErrNotFound
return nil, types.ErrNoSuchContractFn(contractAddr.String()).
Wrapf("address %s", contractAddr.String())
}
rsp := q.keeper.QueryRaw(ctx, contractAddr, req.QueryData)
return &types.QueryRawContractStateResponse{Data: rsp}, nil
Expand Down Expand Up @@ -198,7 +201,8 @@ func (q grpcQuerier) SmartContractState(c context.Context, req *types.QuerySmart
case err != nil:
return nil, err
case bz == nil:
return nil, types.ErrNotFound
return nil, types.ErrNoSuchContractFn(contractAddr.String()).
Wrapf("address %s", contractAddr.String())
}
return &types.QuerySmartContractStateResponse{Data: bz}, nil
}
Expand All @@ -215,7 +219,7 @@ func (q grpcQuerier) Code(c context.Context, req *types.QueryCodeRequest) (*type
case err != nil:
return nil, err
case rsp == nil:
return nil, types.ErrNotFound
return nil, types.ErrNoSuchCodeFn(req.CodeId).Wrapf("code id %d", req.CodeId)
}
return &types.QueryCodeResponse{
CodeInfoResponse: rsp.CodeInfoResponse,
Expand Down Expand Up @@ -254,7 +258,8 @@ func (q grpcQuerier) Codes(c context.Context, req *types.QueryCodesRequest) (*ty
func queryContractInfo(ctx sdk.Context, addr sdk.AccAddress, keeper types.ViewKeeper) (*types.QueryContractInfoResponse, error) {
info := keeper.GetContractInfo(ctx, addr)
if info == nil {
return nil, types.ErrNotFound
return nil, types.ErrNoSuchContractFn(addr.String()).
Wrapf("address %s", addr.String())
}
return &types.QueryContractInfoResponse{
Address: addr.String(),
Expand Down
27 changes: 17 additions & 10 deletions x/wasm/keeper/querier_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,22 @@ func TestQueryAllContractState(t *testing.T) {
}
require.NoError(t, keeper.importContractState(ctx, contractAddr, contractModel))

randomAddr := RandomBech32AccountAddress(t)

q := Querier(keeper)
specs := map[string]struct {
srcQuery *types.QueryAllContractStateRequest
expModelContains []types.Model
expModelContainsNot []types.Model
expErr *sdkErrors.Error
expErr error
}{
"query all": {
srcQuery: &types.QueryAllContractStateRequest{Address: contractAddr.String()},
expModelContains: contractModel,
},
"query all with unknown address": {
srcQuery: &types.QueryAllContractStateRequest{Address: RandomBech32AccountAddress(t)},
expErr: types.ErrNotFound,
srcQuery: &types.QueryAllContractStateRequest{Address: randomAddr},
expErr: types.ErrNoSuchContractFn(randomAddr).Wrapf("address %s", randomAddr),
},
"with pagination offset": {
srcQuery: &types.QueryAllContractStateRequest{
Expand Down Expand Up @@ -99,8 +101,9 @@ func TestQueryAllContractState(t *testing.T) {
for msg, spec := range specs {
t.Run(msg, func(t *testing.T) {
got, err := q.AllContractState(sdk.WrapSDKContext(ctx), spec.srcQuery)
require.True(t, spec.expErr.Is(err), err)

if spec.expErr != nil {
require.Equal(t, spec.expErr.Error(), err.Error())
return
}
for _, exp := range spec.expModelContains {
Expand All @@ -120,6 +123,8 @@ func TestQuerySmartContractState(t *testing.T) {
exampleContract := InstantiateHackatomExampleContract(t, ctx, keepers)
contractAddr := exampleContract.Contract.String()

randomAddr := RandomBech32AccountAddress(t)

q := Querier(keeper)
specs := map[string]struct {
srcAddr sdk.AccAddress
Expand All @@ -140,8 +145,8 @@ func TestQuerySmartContractState(t *testing.T) {
expErr: status.Error(codes.InvalidArgument, "invalid query data"),
},
"query smart with unknown address": {
srcQuery: &types.QuerySmartContractStateRequest{Address: RandomBech32AccountAddress(t), QueryData: []byte(`{"verifier":{}}`)},
expErr: types.ErrNotFound,
srcQuery: &types.QuerySmartContractStateRequest{Address: randomAddr, QueryData: []byte(`{"verifier":{}}`)},
expErr: types.ErrNoSuchContractFn(randomAddr),
},
}
for msg, spec := range specs {
Expand Down Expand Up @@ -213,11 +218,13 @@ func TestQueryRawContractState(t *testing.T) {
}
require.NoError(t, keeper.importContractState(ctx, exampleContract.Contract, contractModel))

randomAddr := RandomBech32AccountAddress(t)

q := Querier(keeper)
specs := map[string]struct {
srcQuery *types.QueryRawContractStateRequest
expData []byte
expErr *sdkErrors.Error
expErr error
}{
"query raw key": {
srcQuery: &types.QueryRawContractStateRequest{Address: contractAddr, QueryData: []byte("foo")},
Expand All @@ -240,15 +247,15 @@ func TestQueryRawContractState(t *testing.T) {
expData: nil,
},
"query raw with unknown address": {
srcQuery: &types.QueryRawContractStateRequest{Address: RandomBech32AccountAddress(t), QueryData: []byte("foo")},
expErr: types.ErrNotFound,
srcQuery: &types.QueryRawContractStateRequest{Address: randomAddr, QueryData: []byte("foo")},
expErr: types.ErrNoSuchContractFn(randomAddr).Wrapf("address %s", randomAddr),
},
}
for msg, spec := range specs {
t.Run(msg, func(t *testing.T) {
got, err := q.RawContractState(sdk.WrapSDKContext(ctx), spec.srcQuery)
require.True(t, spec.expErr.Is(err), err)
if spec.expErr != nil {
assert.Equal(t, spec.expErr.Error(), err.Error())
return
}
assert.Equal(t, spec.expData, got.Data)
Expand Down

0 comments on commit d2b504a

Please sign in to comment.