Skip to content

Commit 7135e8c

Browse files
fbsobreirafbsobreira
committed
[KLC-2355] address review: cog complexity, coverage, error wrapping
- accounts: extract claimErrorResultCode helper used by ClaimStaking and ClaimAllowance, removing the duplicated MaxSupplyExceeded mapping and dropping ClaimStaking under the cog-complexity limit. - nodeAccount: switch errors.New(...+err.Error()) to fmt.Errorf("...: %w", err) so callers can still inspect the underlying error via errors.Is/As. - Tests: cover the previously-untested error branches (ClaimStaking max-supply, GetAccount/GetNextNonce account-not-found and wrap-non-sentinel, SFTGetMeta nil-trie, GetStorageData account-not-found) including wrapped sentinel variants.
1 parent 7cf5835 commit 7135e8c

6 files changed

Lines changed: 347 additions & 14 deletions

File tree

‎core/kapp/accounts/accounts.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1558,13 +1558,7 @@ func (a *accountsKapp) ClaimStaking(sender []byte, tc *transaction.ClaimContract
15581558
gains, err := a.ClaimBalance(transaction.ClaimContract_StakingClaim, assetID, ctx.Block(), ownerAcc, staking, kda, userKDA)
15591559
if err != nil {
15601560
ctx.Receipts().AddError(ctx.ContractID(), common.ErrFieldClaimNotAvailable, err.Error())
1561-
var resultCode transaction.Transaction_TXResultCode
1562-
if errors.Is(err, common.ErrMaxSupplyExceeded) {
1563-
resultCode = transaction.Transaction_MaxSupplyExceeded
1564-
} else {
1565-
resultCode = transaction.Transaction_ClaimError
1566-
}
1567-
return resultCode, err
1561+
return claimErrorResultCode(err, transaction.Transaction_ClaimError), err
15681562
}
15691563

15701564
validClaims := 0
@@ -1685,11 +1679,7 @@ func (a *accountsKapp) ClaimAllowance(sender []byte, tc *transaction.ClaimContra
16851679
gains, err := a.ClaimBalance(transaction.ClaimContract_AllowanceClaim, kdautils.KLVIdentifier, ctx.Block(), ownerAcc, nil, nil, userKDA)
16861680
if err != nil {
16871681
ctx.Receipts().AddError(ctx.ContractID(), common.ErrFieldClaimNotAvailable, err.Error())
1688-
resultCode := transaction.Transaction_ClaimError
1689-
if errors.Is(err, common.ErrMaxSupplyExceeded) {
1690-
resultCode = transaction.Transaction_MaxSupplyExceeded
1691-
}
1692-
return resultCode, err
1682+
return claimErrorResultCode(err, transaction.Transaction_ClaimError), err
16931683
}
16941684

16951685
for key, value := range gains {
@@ -1929,3 +1919,13 @@ func claimAddress(interestType kapps.StakingData_EnumInterestType) []byte {
19291919
// return StakingKApp address
19301920
return kapps.StakingKAppAddress
19311921
}
1922+
1923+
// claimErrorResultCode maps a ClaimBalance failure to a transaction result code.
1924+
// ErrMaxSupplyExceeded surfaces a dedicated code so it can be distinguished from
1925+
// generic claim errors by downstream receipts and clients.
1926+
func claimErrorResultCode(err error, defaultCode transaction.Transaction_TXResultCode) transaction.Transaction_TXResultCode {
1927+
if errors.Is(err, common.ErrMaxSupplyExceeded) {
1928+
return transaction.Transaction_MaxSupplyExceeded
1929+
}
1930+
return defaultCode
1931+
}

‎core/kapp/accounts/accounts_test.go‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"encoding/hex"
66
"errors"
7+
"fmt"
78
"math"
89
"testing"
910

@@ -5503,3 +5504,142 @@ func Test_ClaimStaking_LoadAccountError(t *testing.T) {
55035504
require.Error(t, err)
55045505
assert.Equal(t, transaction.Transaction_LoadAccountError, code)
55055506
}
5507+
5508+
func Test_ClaimStaking_MaxSupplyExceeded(t *testing.T) {
5509+
accountsKapp := setupAccountsKapp(t, config.EnableEpochs{})
5510+
5511+
receiptsCtx := &commonMock.ReceiptsContextStub{}
5512+
ctx := &commonMock.KAppContextStub{
5513+
ReceiptsCalled: func() kapp.ReceiptsContext { return receiptsCtx },
5514+
ContractIDCalled: func() int { return 1 },
5515+
BlockCalled: func() *block.Block {
5516+
return &block.Block{Header: &block.BlockHeader{Timestamp: 1000, Epoch: 1}}
5517+
},
5518+
}
5519+
5520+
_ = accountsKapp.SetKAppController(&kvmStub.KAppControllerStub{
5521+
GetCurrentKAppContextCalled: func() kapp.KappContext { return ctx },
5522+
GetKDAKAppCalled: func() kapp.KDAKapp {
5523+
return &kvmStub.KDAKappStub{
5524+
GetStakingCalled: func(assetID []byte) (state.KAppAccountHandler, *kapps.StakingData, error) {
5525+
return nil, &kapps.StakingData{}, nil
5526+
},
5527+
GetKDACalled: func(assetID []byte) (state.KAppAccountHandler, *kapps.KDAData, error) {
5528+
return nil, &kapps.KDAData{AssetType: kapps.KDAData_Fungible}, nil
5529+
},
5530+
}
5531+
},
5532+
})
5533+
5534+
_ = accountsKapp.SetAccountsCacher(&commonMock.AccountsCacherStub{
5535+
GetExistingUserCalled: func(address []byte) (state.UserAccountHandler, error) {
5536+
return &commonMock.UserAccountHandlerStub{
5537+
GetUserKDACalled: func(assetID, nonce []byte, checkDirtData bool) (*kapps.UserKDA, error) {
5538+
return &kapps.UserKDA{}, nil
5539+
},
5540+
ClaimCalled: func(claimType transaction.ClaimContract_EnumClaimType, assetID []byte, epoch uint32, blockTime int64, staking *kapps.StakingData, kda *kapps.KDAData, userKDA *kapps.UserKDA, forkController core.ForkController) (map[string]int64, error) {
5541+
return nil, common.ErrMaxSupplyExceeded
5542+
},
5543+
}, nil
5544+
},
5545+
})
5546+
5547+
tc := &transaction.ClaimContract{
5548+
ClaimType: transaction.ClaimContract_StakingClaim,
5549+
ID: kdautils.KLVIdentifier,
5550+
}
5551+
5552+
code, err := accountsKapp.ClaimStaking(txSender, tc)
5553+
require.ErrorIs(t, err, common.ErrMaxSupplyExceeded)
5554+
assert.Equal(t, transaction.Transaction_MaxSupplyExceeded, code)
5555+
}
5556+
5557+
func Test_ClaimStaking_MaxSupplyExceeded_Wrapped(t *testing.T) {
5558+
accountsKapp := setupAccountsKapp(t, config.EnableEpochs{})
5559+
wrapped := fmt.Errorf("ctx: %w", common.ErrMaxSupplyExceeded)
5560+
5561+
receiptsCtx := &commonMock.ReceiptsContextStub{}
5562+
ctx := &commonMock.KAppContextStub{
5563+
ReceiptsCalled: func() kapp.ReceiptsContext { return receiptsCtx },
5564+
ContractIDCalled: func() int { return 1 },
5565+
BlockCalled: func() *block.Block {
5566+
return &block.Block{Header: &block.BlockHeader{Timestamp: 1000, Epoch: 1}}
5567+
},
5568+
}
5569+
5570+
_ = accountsKapp.SetKAppController(&kvmStub.KAppControllerStub{
5571+
GetCurrentKAppContextCalled: func() kapp.KappContext { return ctx },
5572+
GetKDAKAppCalled: func() kapp.KDAKapp {
5573+
return &kvmStub.KDAKappStub{
5574+
GetStakingCalled: func(assetID []byte) (state.KAppAccountHandler, *kapps.StakingData, error) {
5575+
return nil, &kapps.StakingData{}, nil
5576+
},
5577+
GetKDACalled: func(assetID []byte) (state.KAppAccountHandler, *kapps.KDAData, error) {
5578+
return nil, &kapps.KDAData{AssetType: kapps.KDAData_Fungible}, nil
5579+
},
5580+
}
5581+
},
5582+
})
5583+
5584+
_ = accountsKapp.SetAccountsCacher(&commonMock.AccountsCacherStub{
5585+
GetExistingUserCalled: func(address []byte) (state.UserAccountHandler, error) {
5586+
return &commonMock.UserAccountHandlerStub{
5587+
GetUserKDACalled: func(assetID, nonce []byte, checkDirtData bool) (*kapps.UserKDA, error) {
5588+
return &kapps.UserKDA{}, nil
5589+
},
5590+
ClaimCalled: func(claimType transaction.ClaimContract_EnumClaimType, assetID []byte, epoch uint32, blockTime int64, staking *kapps.StakingData, kda *kapps.KDAData, userKDA *kapps.UserKDA, forkController core.ForkController) (map[string]int64, error) {
5591+
return nil, wrapped
5592+
},
5593+
}, nil
5594+
},
5595+
})
5596+
5597+
tc := &transaction.ClaimContract{
5598+
ClaimType: transaction.ClaimContract_StakingClaim,
5599+
ID: kdautils.KLVIdentifier,
5600+
}
5601+
5602+
code, err := accountsKapp.ClaimStaking(txSender, tc)
5603+
require.ErrorIs(t, err, common.ErrMaxSupplyExceeded)
5604+
assert.Equal(t, transaction.Transaction_MaxSupplyExceeded, code)
5605+
}
5606+
5607+
func Test_claimErrorResultCode(t *testing.T) {
5608+
t.Parallel()
5609+
5610+
tests := []struct {
5611+
name string
5612+
err error
5613+
fallback transaction.Transaction_TXResultCode
5614+
want transaction.Transaction_TXResultCode
5615+
}{
5616+
{
5617+
name: "direct ErrMaxSupplyExceeded -> MaxSupplyExceeded",
5618+
err: common.ErrMaxSupplyExceeded,
5619+
fallback: transaction.Transaction_ClaimError,
5620+
want: transaction.Transaction_MaxSupplyExceeded,
5621+
},
5622+
{
5623+
name: "wrapped ErrMaxSupplyExceeded -> MaxSupplyExceeded",
5624+
err: errWrap("ctx", common.ErrMaxSupplyExceeded),
5625+
fallback: transaction.Transaction_ClaimError,
5626+
want: transaction.Transaction_MaxSupplyExceeded,
5627+
},
5628+
{
5629+
name: "unrelated error -> fallback",
5630+
err: errors.New("something else"),
5631+
fallback: transaction.Transaction_ClaimError,
5632+
want: transaction.Transaction_ClaimError,
5633+
},
5634+
}
5635+
5636+
for _, tt := range tests {
5637+
t.Run(tt.name, func(t *testing.T) {
5638+
assert.Equal(t, tt.want, claimErrorResultCode(tt.err, tt.fallback))
5639+
})
5640+
}
5641+
}
5642+
5643+
func errWrap(msg string, err error) error {
5644+
return fmt.Errorf("%s: %w", msg, err)
5645+
}

‎core/kapp/systemAccount/systemAcount_test.go‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
package systemAccount
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"testing"
7+
8+
"github.com/klever-io/klever-go/common"
9+
commonMock "github.com/klever-io/klever-go/common/mock"
10+
"github.com/klever-io/klever-go/data/state"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
)
14+
15+
func TestSFTGetMeta_NilTrieReturnsEmptyMeta(t *testing.T) {
16+
t.Parallel()
17+
18+
cases := []struct {
19+
name string
20+
err error
21+
}{
22+
{"direct ErrNilTrie", common.ErrNilTrie},
23+
{"wrapped ErrNilTrie", fmt.Errorf("load: %w", common.ErrNilTrie)},
24+
}
25+
for _, c := range cases {
26+
t.Run(c.name, func(t *testing.T) {
27+
s := &systemAccountKApp{}
28+
_ = s.SetAccountsCacher(&commonMock.AccountsCacherStub{
29+
LoadKAppCalled: func(address []byte) (state.KAppAccountHandler, error) {
30+
return nil, c.err
31+
},
32+
})
33+
34+
meta, err := s.SFTGetMeta([]byte("ASSET"), nil)
35+
require.NoError(t, err)
36+
require.NotNil(t, meta)
37+
require.NotNil(t, meta.Metadata)
38+
})
39+
}
40+
}
41+
42+
func TestSFTGetMeta_OtherErrorPropagates(t *testing.T) {
43+
t.Parallel()
44+
45+
other := errors.New("disk failure")
46+
s := &systemAccountKApp{}
47+
_ = s.SetAccountsCacher(&commonMock.AccountsCacherStub{
48+
LoadKAppCalled: func(address []byte) (state.KAppAccountHandler, error) {
49+
return nil, other
50+
},
51+
})
52+
53+
meta, err := s.SFTGetMeta([]byte("ASSET"), nil)
54+
assert.Nil(t, meta)
55+
require.Error(t, err)
56+
require.True(t, errors.Is(err, other))
57+
}

‎core/process/smartContract/hooks/blockChainHook_test.go‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"testing"
99
"time"
1010

11+
"github.com/klever-io/klever-go/common"
1112
commonMock "github.com/klever-io/klever-go/common/mock"
1213
"github.com/klever-io/klever-go/config"
1314
"github.com/klever-io/klever-go/core"
@@ -34,6 +35,7 @@ import (
3435
"github.com/klever-io/klever-go/storage/memorydb"
3536
"github.com/klever-io/klever-go/storage/storageUnit"
3637
"github.com/klever-io/klever-go/tools/marshal"
38+
"github.com/klever-io/klever-go/vmcommon"
3739
"github.com/stretchr/testify/assert"
3840
)
3941

@@ -714,3 +716,48 @@ func TestBlockChainHookImpl_GetKDAToken(t *testing.T) {
714716
})
715717
}
716718
}
719+
720+
// blockChainHookCounterPassthroughStub is a minimal BlockChainHookCounter that
721+
// reports no errors and no-ops everything else, used to isolate higher-layer
722+
// branches under test.
723+
type blockChainHookCounterPassthroughStub struct{}
724+
725+
func (s *blockChainHookCounterPassthroughStub) ProcessCrtNumberOfTrieReadsCounter() error {
726+
return nil
727+
}
728+
func (s *blockChainHookCounterPassthroughStub) ProcessMaxBuiltInCounters(_ *vmcommon.ContractCallInput) error {
729+
return nil
730+
}
731+
func (s *blockChainHookCounterPassthroughStub) ResetCounters() {}
732+
func (s *blockChainHookCounterPassthroughStub) SetMaximumValues(_ map[string]uint64) {}
733+
func (s *blockChainHookCounterPassthroughStub) GetCounterValues() map[string]uint64 { return nil }
734+
func (s *blockChainHookCounterPassthroughStub) IsInterfaceNil() bool { return s == nil }
735+
736+
func TestGetStorageData_AccNotFoundReturnsEmpty(t *testing.T) {
737+
t.Parallel()
738+
739+
cases := []struct {
740+
name string
741+
err error
742+
}{
743+
{"direct ErrAccNotFound", common.ErrAccNotFound},
744+
{"wrapped ErrAccNotFound", fmt.Errorf("load: %w", common.ErrAccNotFound)},
745+
}
746+
for _, c := range cases {
747+
t.Run(c.name, func(t *testing.T) {
748+
bh := &BlockChainHookImpl{
749+
counter: &blockChainHookCounterPassthroughStub{},
750+
accountsCacher: &commonMock.AccountsCacherStub{
751+
GetExistingUserCalled: func(address []byte) (state.UserAccountHandler, error) {
752+
return nil, c.err
753+
},
754+
},
755+
}
756+
757+
value, code, err := bh.GetStorageData([]byte("addr"), []byte("key"))
758+
assert.NoError(t, err)
759+
assert.Equal(t, uint32(0), code)
760+
assert.Equal(t, []byte{}, value)
761+
})
762+
}
763+
}

‎node/nodeAccount.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package node
22

33
import (
44
"errors"
5+
"fmt"
56

67
"github.com/klever-io/klever-go/common"
78
"github.com/klever-io/klever-go/core"
@@ -30,7 +31,7 @@ func (n *Node) GetAccount(address string) (state.UserAccountHandler, error) {
3031
if errors.Is(err, common.ErrAccNotFound) {
3132
return state.NewUserAccount(addr)
3233
}
33-
return nil, errors.New("could not fetch sender address from provided param: " + err.Error())
34+
return nil, fmt.Errorf("could not fetch sender address from provided param: %w", err)
3435
}
3536

3637
account, ok := n.castAccountToUserAccount(accWrp)
@@ -73,7 +74,7 @@ func (n *Node) GetNextNonce(address string) (uint64, uint64, uint64, error) {
7374
if errors.Is(err, common.ErrAccNotFound) {
7475
return 0, 0, 0, nil
7576
}
76-
return 0, 0, 0, errors.New("could not fetch sender address from provided param: " + err.Error())
77+
return 0, 0, 0, fmt.Errorf("could not fetch sender address from provided param: %w", err)
7778
}
7879

7980
accNonce := accWrp.GetNonce()

0 commit comments

Comments
 (0)