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

refactor(panic): remove unused or unnecessary panic code #171

Merged
merged 1 commit into from
Oct 8, 2024
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
24 changes: 0 additions & 24 deletions client/x/evmstaking/types/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,6 @@ package types

import (
"fmt"

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

"github.com/piplabs/story/lib/errors"
)

// Staking params default values.
Expand Down Expand Up @@ -35,26 +31,6 @@ func DefaultParams() Params {
)
}

// unmarshal the current params value from store key or panic.
func MustUnmarshalParams(cdc *codec.LegacyAmino, value []byte) Params {
params, err := UnmarshalParams(cdc, value)
if err != nil {
panic(err)
}

return params
}

// unmarshal the current params value from store key.
func UnmarshalParams(cdc *codec.LegacyAmino, value []byte) (params Params, err error) {
err = cdc.Unmarshal(value, &params)
if err != nil {
return params, errors.Wrap(err, "unmarshal params")
}

return params, nil
}

func ValidateMaxWithdrawalPerBlock(v uint32) error {
if v == 0 {
return fmt.Errorf("max withdrawal per block must be positive: %d", v)
Expand Down
82 changes: 0 additions & 82 deletions client/x/evmstaking/types/params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,88 +37,6 @@ func (suite *ParamsTestSuite) TestDefaultParams() {
require.Equal(types.DefaultMinPartialWithdrawalAmount, params.MinPartialWithdrawalAmount)
}

func (suite *ParamsTestSuite) TestMustUnmarshalParams() {
require := suite.Require()
maxWithdrawalPerBlock, maxSweepPerBlock, minPartialWithdrawalAmount := uint32(1), uint32(2), uint64(3)
params := types.NewParams(maxWithdrawalPerBlock, maxSweepPerBlock, minPartialWithdrawalAmount)

tcs := []struct {
name string
input []byte
expected types.Params
expectPanic bool
}{
{
name: "Unmarshal valid params bytes",
input: suite.encConf.Codec.MustMarshal(&params),
expected: types.Params{
MaxWithdrawalPerBlock: maxWithdrawalPerBlock,
MaxSweepPerBlock: maxSweepPerBlock,
MinPartialWithdrawalAmount: minPartialWithdrawalAmount,
},
},
{
name: "Unmarshal invalid params bytes",
input: []byte{0x1, 0x2, 0x3},
expectPanic: true,
},
}

for _, tc := range tcs {
suite.Run(tc.name, func() {
if tc.expectPanic {
require.Panics(func() {
types.MustUnmarshalParams(suite.encConf.Amino, tc.input)
})
} else {
params := types.MustUnmarshalParams(suite.encConf.Amino, tc.input)
require.Equal(tc.expected, params)
}
})
}
}

func (suite *ParamsTestSuite) TestUnmarshalParams() {
require := suite.Require()
maxWithdrawalPerBlock, maxSweepPerBlock, minPartialWithdrawalAmount := uint32(1), uint32(2), uint64(3)
params := types.NewParams(maxWithdrawalPerBlock, maxSweepPerBlock, minPartialWithdrawalAmount)

tcs := []struct {
name string
input []byte
expected types.Params
expectedError string
}{
{
name: "Unmarshal valid params bytes",
input: suite.encConf.Codec.MustMarshal(&params),
expected: types.Params{
MaxWithdrawalPerBlock: maxWithdrawalPerBlock,
MaxSweepPerBlock: maxSweepPerBlock,
MinPartialWithdrawalAmount: minPartialWithdrawalAmount,
},
},
{
name: "Unmarshal invalid params bytes",
input: []byte{0x1, 0x2, 0x3},
expectedError: "unmarshal params",
},
}

for _, tc := range tcs {
suite.Run(tc.name, func() {
params, err := types.UnmarshalParams(suite.encConf.Amino, tc.input)
if tc.expectedError != "" {
require.Error(err)
require.Contains(err.Error(), tc.expectedError)
} else {
require.NoError(err)
require.Equal(tc.expected, params)
}
})
}
}

func (suite *ParamsTestSuite) TestValidateMaxWithdrawalPerBlock() {
require := suite.Require()

Expand Down
29 changes: 0 additions & 29 deletions client/x/evmstaking/types/withdraw.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@ import (
"strings"

"cosmossdk.io/core/address"

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

"github.com/piplabs/story/lib/errors"
)

// Withdrawals is a collection of Withdrawal.
Expand Down Expand Up @@ -48,28 +44,3 @@ func NewWithdrawalFromMsg(msg *MsgAddWithdrawal) Withdrawal {
Amount: msg.Withdrawal.Amount,
}
}

func MustMarshalWithdrawal(cdc codec.BinaryCodec, withdrawal *Withdrawal) []byte {
return cdc.MustMarshal(withdrawal)
}

// MustUnmarshalWithdrawal return the unmarshaled withdrawal from bytes.
// Panics if fails.
func MustUnmarshalWithdrawal(cdc codec.BinaryCodec, value []byte) Withdrawal {
withdrawal, err := UnmarshalWithdrawal(cdc, value)
if err != nil {
panic(err)
}

return withdrawal
}

// UnmarshalWithdrawal returns the withdrawal.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't use this? Interesting, wonder why we have it in the first place

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I did not find any usage in the code base.

func UnmarshalWithdrawal(cdc codec.BinaryCodec, value []byte) (withdrawal Withdrawal, err error) {
err = cdc.Unmarshal(value, &withdrawal)
if err != nil {
return withdrawal, errors.Wrap(err, "unmarshal withdrawal")
}

return withdrawal, nil
}
87 changes: 0 additions & 87 deletions client/x/evmstaking/types/withdraw_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,93 +109,6 @@ func (suite *WithdrawTestSuite) TestNewWithdrawalFromMsg() {
require.Equal(withdrawal, w, "NewWithdrawalFromMsg should return the same withdrawal")
}

func (suite *WithdrawTestSuite) TestMustMarshalWithdraw() {
require := suite.Require()
withdrawal := types.NewWithdrawal(1, suite.delAddr, suite.valAddr, suite.evmAddr.String(), 1)
require.NotPanics(func() {
marshaled := types.MustMarshalWithdrawal(suite.encConf.Codec, &withdrawal)
require.NotNil(marshaled, "MarshalWithdrawal should not return nil")
})
}

func (suite *WithdrawTestSuite) TestUnmarshalWithdraw() {
require := suite.Require()
withdrawal := types.NewWithdrawal(1, suite.delAddr, suite.valAddr, suite.evmAddr.String(), 1)

testCases := []struct {
name string
input []byte
expectedResult types.Withdrawal
expectError bool
}{
{
name: "Unmarshal valid withdrawal bytes",
input: types.MustMarshalWithdrawal(suite.encConf.Codec, &withdrawal),
expectedResult: types.NewWithdrawal(1, suite.delAddr, suite.valAddr, suite.evmAddr.String(), 1),
expectError: false,
},
{
name: "Unmarshal invalid withdrawal bytes",
input: []byte{1},
expectedResult: types.Withdrawal{}, // Expecting an empty struct since it will fail
expectError: true,
},
}

// Iterate over test cases
for _, tc := range testCases {
suite.Run(tc.name, func() {
result, err := types.UnmarshalWithdrawal(suite.encConf.Codec, tc.input)
if tc.expectError {
require.Error(err)
} else {
require.NoError(err)
require.Equal(tc.expectedResult, result, "UnmarshalWithdrawal should return the correct withdrawal")
}
})
}
}

func (suite *WithdrawTestSuite) TestMustUnmarshalWithdraw() {
require := suite.Require()
withdrawal := types.NewWithdrawal(1, suite.delAddr, suite.valAddr, suite.evmAddr.String(), 1)

testCases := []struct {
name string
input []byte
expected types.Withdrawal
expectPanic bool
}{
{
name: "Unmarshal valid withdrawal bytes",
input: types.MustMarshalWithdrawal(suite.encConf.Codec, &withdrawal),
expected: types.NewWithdrawal(1, suite.delAddr, suite.valAddr, suite.evmAddr.String(), 1),
expectPanic: false,
},
{
name: "Unmarshal invalid withdrawal bytes - panic",
input: []byte{1},
expectPanic: true,
},
}

// Iterate over test cases
for _, tc := range testCases {
suite.Run(tc.name, func() {
if tc.expectPanic {
require.Panics(func() {
types.MustUnmarshalWithdrawal(suite.encConf.Codec, tc.input)
})
} else {
require.NotPanics(func() {
result := types.MustUnmarshalWithdrawal(suite.encConf.Codec, tc.input)
require.Equal(tc.expected, result, "MustUnmarshalWithdrawal should return the correct withdrawal")
})
}
})
}
}

func TestWithdrawalTestSuite(t *testing.T) {
t.Parallel()
suite.Run(t, new(WithdrawTestSuite))
Expand Down
2 changes: 1 addition & 1 deletion lib/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func New(msg string, attrs ...any) error {
//nolint:inamedparam // This function does custom wrapping and errors.
func Wrap(err error, msg string, attrs ...any) error {
if err == nil {
panic("wrap nil error")
return nil
}

// Support error types that do their own wrapping.
Expand Down
71 changes: 0 additions & 71 deletions lib/promutil/resetgauge.go

This file was deleted.

Loading
Loading