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.

Sign up for GitHub

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: returned errors instead of panicing in auth #16212

Closed
wants to merge 4 commits into from
Closed

chore: returned errors instead of panicing in auth #16212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1a5b59c
returned errors instead of panicing in auth
chandiniv1 May 18, 2023
cf23cb3
changed the method signatures in all modules
chandiniv1 May 18, 2023
024082e
made changes in the mocks
chandiniv1 May 19, 2023
314910f
handled errors
chandiniv1 May 20, 2023
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
9 changes: 6 additions & 3 deletions baseapp/block_gas_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,8 @@ func TestBaseApp_BlockGas(t *testing.T) {
err = bankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr1, feeAmount)
require.NoError(t, err)
require.Equal(t, feeCoin.Amount, bankKeeper.GetBalance(ctx, addr1, feeCoin.Denom).Amount)
seq := accountKeeper.GetAccount(ctx, addr1).GetSequence()
getAccount,_:=accountKeeper.GetAccount(ctx,addr1)
seq := getAccount.GetSequence()
require.Equal(t, uint64(0), seq)

// msg and signatures
Expand All @@ -153,7 +154,8 @@ func TestBaseApp_BlockGas(t *testing.T) {
txBuilder.SetFeeAmount(feeAmount)
txBuilder.SetGasLimit(txtypes.MaxGasWanted) // tx validation checks that gasLimit can't be bigger than this

senderAccountNumber := accountKeeper.GetAccount(ctx, addr1).GetAccountNumber()
getAcc,_:=accountKeeper.GetAccount(ctx, addr1)
senderAccountNumber := getAcc.GetAccountNumber()
privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{senderAccountNumber}, []uint64{0}
_, txBytes, err := createTestTx(txConfig, txBuilder, privs, accNums, accSeqs, ctx.ChainID())
require.NoError(t, err)
Expand Down Expand Up @@ -187,7 +189,8 @@ func TestBaseApp_BlockGas(t *testing.T) {
// tx fee is always deducted
require.Equal(t, int64(0), bankKeeper.GetBalance(ctx, addr1, feeCoin.Denom).Amount.Int64())
// sender's sequence is always increased
seq = accountKeeper.GetAccount(ctx, addr1).GetSequence()
getAccc,_:=accountKeeper.GetAccount(ctx, addr1)
seq = getAccc.GetSequence()
require.NoError(t, err)
require.Equal(t, uint64(1), seq)
})
Expand Down
5 changes: 4 additions & 1 deletion docs/architecture/adr-011-generalize-genesis-accounts.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ func InitGenesis(ctx sdk.Context, ak AccountKeeper, data GenesisState) {
// load the accounts
for _, a := range data.Accounts {
acc := ak.NewAccount(ctx, a) // set account number
ak.SetAccount(ctx, acc)
err:=ak.SetAccount(ctx, acc)
if err!=nil{
panic(err)
}
}
}

Expand Down
30 changes: 21 additions & 9 deletions types/query/filtered_pagination_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,11 @@ func (s *paginationTestSuite) TestFilteredPaginations() {

balances = balances.Sort()
addr1 := sdk.AccAddress([]byte("addr1"))
acc1 := s.accountKeeper.NewAccountWithAddress(s.ctx, addr1)
s.accountKeeper.SetAccount(s.ctx, acc1)
acc1,_ := s.accountKeeper.NewAccountWithAddress(s.ctx, addr1)
err:=s.accountKeeper.SetAccount(s.ctx, acc1)
if err!=nil{
fmt.Println(err)
}
s.Require().NoError(testutil.FundAccount(s.ctx, s.bankKeeper, addr1, balances))
store := s.ctx.KVStore(s.app.UnsafeFindStoreKey(types.StoreKey))

Expand Down Expand Up @@ -104,8 +107,11 @@ func (s *paginationTestSuite) TestReverseFilteredPaginations() {

balances = balances.Sort()
addr1 := sdk.AccAddress([]byte("addr1"))
acc1 := s.accountKeeper.NewAccountWithAddress(s.ctx, addr1)
s.accountKeeper.SetAccount(s.ctx, acc1)
acc1,_ := s.accountKeeper.NewAccountWithAddress(s.ctx, addr1)
err:=s.accountKeeper.SetAccount(s.ctx, acc1)
if err!=nil{
fmt.Println(err)
}
s.Require().NoError(testutil.FundAccount(s.ctx, s.bankKeeper, addr1, balances))
store := s.ctx.KVStore(s.app.UnsafeFindStoreKey(types.StoreKey))

Expand Down Expand Up @@ -182,9 +188,12 @@ func (s *paginationTestSuite) TestFilteredPaginate() {

balances = balances.Sort()
addr1 := sdk.AccAddress([]byte("addr1"))
acc1 := s.accountKeeper.NewAccountWithAddress(s.ctx, addr1)
s.accountKeeper.SetAccount(s.ctx, acc1)
err := testutil.FundAccount(s.ctx, s.bankKeeper, addr1, balances)
acc1 ,_:= s.accountKeeper.NewAccountWithAddress(s.ctx, addr1)
err:=s.accountKeeper.SetAccount(s.ctx, acc1)
if err!=nil{
fmt.Println(err)
}
err = testutil.FundAccount(s.ctx, s.bankKeeper, addr1, balances)
if err != nil { // should return no error
fmt.Println(err)
}
Expand Down Expand Up @@ -258,8 +267,11 @@ func (s *paginationTestSuite) TestFilteredPaginationsNextKey() {

balances = balances.Sort()
addr1 := sdk.AccAddress([]byte("addr1"))
acc1 := s.accountKeeper.NewAccountWithAddress(s.ctx, addr1)
s.accountKeeper.SetAccount(s.ctx, acc1)
acc1,_ := s.accountKeeper.NewAccountWithAddress(s.ctx, addr1)
err:=s.accountKeeper.SetAccount(s.ctx, acc1)
if err!=nil{
fmt.Println(err)
}
s.Require().NoError(testutil.FundAccount(s.ctx, s.bankKeeper, addr1, balances))
store := s.ctx.KVStore(s.app.UnsafeFindStoreKey(types.StoreKey))

Expand Down
7 changes: 5 additions & 2 deletions types/query/fuzz_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,11 @@ func FuzzPagination(f *testing.F) {

balances = balances.Sort()
addr1 := sdk.AccAddress([]byte("addr1"))
acc1 := suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr1)
suite.accountKeeper.SetAccount(suite.ctx, acc1)
acc1,_ := suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr1)
err1:=suite.accountKeeper.SetAccount(suite.ctx, acc1)
if err1!=nil{
f.Fatal(err1)
}
err := testutil.FundAccount(suite.ctx, suite.bankKeeper, addr1, balances)
if err != nil { // should return no error
f.Fatal(err)
Expand Down
21 changes: 15 additions & 6 deletions types/query/pagination_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,11 @@ func (s *paginationTestSuite) TestPagination() {

balances = balances.Sort()
addr1 := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address())
acc1 := s.accountKeeper.NewAccountWithAddress(s.ctx, addr1)
s.accountKeeper.SetAccount(s.ctx, acc1)
acc1,_ := s.accountKeeper.NewAccountWithAddress(s.ctx, addr1)
err:=s.accountKeeper.SetAccount(s.ctx, acc1)
if err!=nil{
fmt.Println(err)
}
s.Require().NoError(testutil.FundAccount(s.ctx, s.bankKeeper, addr1, balances))

s.T().Log("verify empty page request results a max of defaultLimit records and counts total records")
Expand Down Expand Up @@ -227,8 +230,11 @@ func (s *paginationTestSuite) TestReversePagination() {

balances = balances.Sort()
addr1 := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address())
acc1 := s.accountKeeper.NewAccountWithAddress(s.ctx, addr1)
s.accountKeeper.SetAccount(s.ctx, acc1)
acc1 ,_:= s.accountKeeper.NewAccountWithAddress(s.ctx, addr1)
err:=s.accountKeeper.SetAccount(s.ctx, acc1)
if err!=nil{
fmt.Println(err)
}
s.Require().NoError(testutil.FundAccount(s.ctx, s.bankKeeper, addr1, balances))

s.T().Log("verify paginate with custom limit and countTotal, Reverse false")
Expand Down Expand Up @@ -346,8 +352,11 @@ func (s *paginationTestSuite) TestPaginate() {

balances = balances.Sort()
addr1 := sdk.AccAddress([]byte("addr1"))
acc1 := s.accountKeeper.NewAccountWithAddress(s.ctx, addr1)
s.accountKeeper.SetAccount(s.ctx, acc1)
acc1,_ := s.accountKeeper.NewAccountWithAddress(s.ctx, addr1)
err1:=s.accountKeeper.SetAccount(s.ctx, acc1)
if err1!=nil{
fmt.Println(err1)
}
err := testutil.FundAccount(s.ctx, s.bankKeeper, addr1, balances)
if err != nil { // should return no error
fmt.Println(err)
Expand Down
16 changes: 8 additions & 8 deletions x/auth/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,25 +190,25 @@ all fields of all accounts, and to iterate over all stored accounts.
// AccountKeeperI is the interface contract that x/auth's keeper implements.
type AccountKeeperI interface {
// Return a new account with the next account number and the specified address. Does not save the new account to the store.
NewAccountWithAddress(sdk.Context, sdk.AccAddress) types.AccountI
NewAccountWithAddress(sdk.Context, sdk.AccAddress) (types.AccountI,error)

// Return a new account with the next account number. Does not save the new account to the store.
NewAccount(sdk.Context, types.AccountI) types.AccountI
NewAccount(sdk.Context, types.AccountI) (types.AccountI,error)

// Check if an account exists in the store.
HasAccount(sdk.Context, sdk.AccAddress) bool
HasAccount(sdk.Context, sdk.AccAddress) (bool,error)

// Retrieve an account from the store.
GetAccount(sdk.Context, sdk.AccAddress) types.AccountI
GetAccount(sdk.Context, sdk.AccAddress) (types.AccountI,error)

// Set an account in the store.
SetAccount(sdk.Context, types.AccountI)
SetAccount(sdk.Context, types.AccountI) error

// Remove an account from the store.
RemoveAccount(sdk.Context, types.AccountI)
RemoveAccount(sdk.Context, types.AccountI) error

// Iterate over all accounts, calling the provided function. Stop iteration when it returns true.
IterateAccounts(sdk.Context, func(types.AccountI) bool)
IterateAccounts(sdk.Context, func(types.AccountI) bool) error

// Fetch the public key of an account at a specified address
GetPubKey(sdk.Context, sdk.AccAddress) (crypto.PubKey, error)
Expand All @@ -217,7 +217,7 @@ type AccountKeeperI interface {
GetSequence(sdk.Context, sdk.AccAddress) (uint64, error)

// Fetch the next account number, and increment the internal counter.
NextAccountNumber(sdk.Context) uint64
NextAccountNumber(sdk.Context) (uint64,error)
}
```

Expand Down
26 changes: 16 additions & 10 deletions x/auth/ante/ante_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,9 @@ func TestAnteHandlerSigErrors(t *testing.T) {
{
"save the first account, but second is still unrecognized",
func(suite *AnteTestSuite) TestCaseArgs {
suite.accountKeeper.SetAccount(suite.ctx, suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr0))
newAcc,_:=suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr0)
suite.accountKeeper.SetAccount(suite.ctx, newAcc)

suite.bankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)

privs, accNums, accSeqs := []cryptotypes.PrivKey{priv0, priv1, priv2}, []uint64{0, 1, 2}, []uint64{0, 0, 0}
Expand All @@ -187,9 +189,13 @@ func TestAnteHandlerSigErrors(t *testing.T) {
{
"save all the accounts, should pass",
func(suite *AnteTestSuite) TestCaseArgs {
suite.accountKeeper.SetAccount(suite.ctx, suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr0))
suite.accountKeeper.SetAccount(suite.ctx, suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr1))
suite.accountKeeper.SetAccount(suite.ctx, suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr2))
newAcc1,_:=suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr0)
newAcc2,_:=suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr1)
newAcc3,_:=suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr2)

suite.accountKeeper.SetAccount(suite.ctx,newAcc1 )
suite.accountKeeper.SetAccount(suite.ctx, newAcc2)
suite.accountKeeper.SetAccount(suite.ctx, newAcc3)
suite.bankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)

privs, accNums, accSeqs := []cryptotypes.PrivKey{priv0, priv1, priv2}, []uint64{1, 2, 3}, []uint64{0, 0, 0}
Expand Down Expand Up @@ -1046,7 +1052,7 @@ func TestAnteHandlerSetPubKey(t *testing.T) {
require.NoError(t, err)

// Make sure public key has been set from previous test.
acc0 := suite.accountKeeper.GetAccount(suite.ctx, accs[0].acc.GetAddress())
acc0,_ := suite.accountKeeper.GetAccount(suite.ctx, accs[0].acc.GetAddress())
require.Equal(t, acc0.GetPubKey(), accs[0].priv.PubKey())

return TestCaseArgs{
Expand Down Expand Up @@ -1080,7 +1086,7 @@ func TestAnteHandlerSetPubKey(t *testing.T) {
suite.bankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)

// Make sure public key has not been set from previous test.
acc1 := suite.accountKeeper.GetAccount(suite.ctx, accs[1].acc.GetAddress())
acc1,_ := suite.accountKeeper.GetAccount(suite.ctx, accs[1].acc.GetAddress())
require.Nil(t, acc1.GetPubKey())

privs, accNums, accSeqs := []cryptotypes.PrivKey{accs[1].priv}, []uint64{accs[1].acc.GetAccountNumber()}, []uint64{accs[1].acc.GetSequence()}
Expand All @@ -1102,7 +1108,7 @@ func TestAnteHandlerSetPubKey(t *testing.T) {
require.True(t, errors.Is(err, sdkerrors.ErrNoSignatures))

// Make sure public key has not been set.
acc1 = suite.accountKeeper.GetAccount(suite.ctx, accs[1].acc.GetAddress())
acc1,_ = suite.accountKeeper.GetAccount(suite.ctx, accs[1].acc.GetAddress())
require.Nil(t, acc1.GetPubKey())

// Set incorrect accSeq, to generate incorrect signature.
Expand All @@ -1126,7 +1132,7 @@ func TestAnteHandlerSetPubKey(t *testing.T) {
suite.bankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)

// Make sure public key has not been set from previous test.
acc1 := suite.accountKeeper.GetAccount(suite.ctx, accs[1].acc.GetAddress())
acc1,_ := suite.accountKeeper.GetAccount(suite.ctx, accs[1].acc.GetAddress())
require.Nil(t, acc1.GetPubKey())

privs, accNums, accSeqs := []cryptotypes.PrivKey{accs[1].priv}, []uint64{accs[1].acc.GetAccountNumber()}, []uint64{accs[1].acc.GetSequence()}
Expand All @@ -1148,7 +1154,7 @@ func TestAnteHandlerSetPubKey(t *testing.T) {
require.True(t, errors.Is(err, sdkerrors.ErrNoSignatures))

// Make sure public key has not been set.
acc1 = suite.accountKeeper.GetAccount(suite.ctx, accs[1].acc.GetAddress())
acc1,_ = suite.accountKeeper.GetAccount(suite.ctx, accs[1].acc.GetAddress())
require.Nil(t, acc1.GetPubKey())

// Set incorrect accSeq, to generate incorrect signature.
Expand All @@ -1159,7 +1165,7 @@ func TestAnteHandlerSetPubKey(t *testing.T) {

// Make sure public key has been set, as SetPubKeyDecorator
// is called before all signature verification decorators.
acc1 = suite.accountKeeper.GetAccount(suite.ctx, accs[1].acc.GetAddress())
acc1,_ = suite.accountKeeper.GetAccount(suite.ctx, accs[1].acc.GetAddress())
require.Equal(t, acc1.GetPubKey(), accs[1].priv.PubKey())
suite.bankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)

Expand Down
2 changes: 1 addition & 1 deletion x/auth/ante/expected_keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
type AccountKeeper interface {
GetParams(ctx context.Context) (params types.Params)
GetAccount(ctx context.Context, addr sdk.AccAddress) sdk.AccountI
SetAccount(ctx context.Context, acc sdk.AccountI)
SetAccount(ctx context.Context, acc sdk.AccountI) error
GetModuleAddress(moduleName string) sdk.AccAddress
}

Expand Down
2 changes: 1 addition & 1 deletion x/auth/ante/feegrant_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func TestDeductFeesNoDelegation(t *testing.T) {
fee := sdk.NewCoins(sdk.NewInt64Coin("atom", tc.fee))
msgs := []sdk.Msg{testdata.NewTestMsg(signer.acc.GetAddress())}

acc := suite.accountKeeper.GetAccount(suite.ctx, signer.acc.GetAddress())
acc,_ := suite.accountKeeper.GetAccount(suite.ctx, signer.acc.GetAddress())
privs, accNums, seqs := []cryptotypes.PrivKey{signer.priv}, []uint64{0}, []uint64{0}
if acc != nil {
accNums, seqs = []uint64{acc.GetAccountNumber()}, []uint64{acc.GetSequence()}
Expand Down
10 changes: 8 additions & 2 deletions x/auth/ante/sigverify.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,10 @@ func (spkd SetPubKeyDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b
if err != nil {
return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, err.Error())
}
spkd.ak.SetAccount(ctx, acc)
err1:=spkd.ak.SetAccount(ctx, acc)
if err1!=nil{
return ctx,err1
}
}

// Also emit the following events, so that txs can be indexed by these
Expand Down Expand Up @@ -354,7 +357,10 @@ func (isd IncrementSequenceDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim
panic(err)
}

isd.ak.SetAccount(ctx, acc)
err:=isd.ak.SetAccount(ctx, acc)
if err!=nil{
panic(err)
}
}

return next(ctx, tx, simulate)
Expand Down
8 changes: 4 additions & 4 deletions x/auth/ante/sigverify_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestSetPubKey(t *testing.T) {
msgs := make([]sdk.Msg, len(addrs))
// set accounts and create msg for each address
for i, addr := range addrs {
acc := suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr)
acc,_:= suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr)
require.NoError(t, acc.SetAccountNumber(uint64(i)))
suite.accountKeeper.SetAccount(suite.ctx, acc)
msgs[i] = testdata.NewTestMsg(addr)
Expand Down Expand Up @@ -155,7 +155,7 @@ func TestSigVerification(t *testing.T) {
msgs := make([]sdk.Msg, len(addrs))
// set accounts and create msg for each address
for i, addr := range addrs {
acc := suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr)
acc ,_:= suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr)
require.NoError(t, acc.SetAccountNumber(uint64(i)))
suite.accountKeeper.SetAccount(suite.ctx, acc)
msgs[i] = testdata.NewTestMsg(addr)
Expand Down Expand Up @@ -274,7 +274,7 @@ func runSigDecorators(t *testing.T, params types.Params, _ bool, privs ...crypto
// set accounts and create msg for each address
for i, priv := range privs {
addr := sdk.AccAddress(priv.PubKey().Address())
acc := suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr)
acc,_ := suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr)
require.NoError(t, acc.SetAccountNumber(uint64(i)))
suite.accountKeeper.SetAccount(suite.ctx, acc)
msgs[i] = testdata.NewTestMsg(addr)
Expand Down Expand Up @@ -313,7 +313,7 @@ func TestIncrementSequenceDecorator(t *testing.T) {
suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder()

priv, _, addr := testdata.KeyTestPubAddr()
acc := suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr)
acc ,_:= suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr)
require.NoError(t, acc.SetAccountNumber(uint64(50)))
suite.accountKeeper.SetAccount(suite.ctx, acc)

Expand Down
2 changes: 1 addition & 1 deletion x/auth/ante/testutil_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func (suite *AnteTestSuite) CreateTestAccounts(numAccs int) []TestAccount {

for i := 0; i < numAccs; i++ {
priv, _, addr := testdata.KeyTestPubAddr()
acc := suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr)
acc,_ := suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr)
acc.SetAccountNumber(uint64(i + 1000))
suite.accountKeeper.SetAccount(suite.ctx, acc)
accounts = append(accounts, TestAccount{acc, priv})
Expand Down
Loading