Skip to content

Commit

Permalink
fix: remove potential runtime panic in x/feegrant (#720)
Browse files Browse the repository at this point in the history
* Remove potential runtime panic

* Update `CHANGELOG.md`

* Fix for codecov

* Add test

* Fix for reviews

* Add test case

* Fix names
  • Loading branch information
ulbqb authored Oct 19, 2022
1 parent b13ab12 commit 0ade7e4
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (global) [\#694](https://github.com/line/lbm-sdk/pull/694) replace deprecated functions since go 1.16 or 1.17
* (x/bankplus) [\#705](https://github.com/line/lbm-sdk/pull/705) add missing blockedAddr checking in bankplus
* (x/foundation) [\#712](https://github.com/line/lbm-sdk/pull/712) fix x/foundation EndBlocker
* (x/feegrant) [\#720](https://github.com/line/lbm-sdk/pull/720) remove potential runtime panic in x/feegrant
* (baseapp) [\#724](https://github.com/line/lbm-sdk/pull/724) add checking pubkey type from validator params
* (x/staking) [\#726](https://github.com/line/lbm-sdk/pull/726) check allowedList size in StakeAuthorization.Accept()
* (x/staking) [\#728](https://github.com/line/lbm-sdk/pull/728) fix typo in unbondingToUnbonded() panic
Expand Down
9 changes: 6 additions & 3 deletions x/feegrant/filtered_fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,14 @@ func (a *AllowedMsgAllowance) GetAllowance() (FeeAllowanceI, error) {
// SetAllowance sets allowed fee allowance.
func (a *AllowedMsgAllowance) SetAllowance(allowance FeeAllowanceI) error {
var err error
a.Allowance, err = types.NewAnyWithValue(allowance.(proto.Message))
if err != nil {
protoAllowance, ok := allowance.(proto.Message)
if !ok {
return sdkerrors.Wrapf(sdkerrors.ErrPackAny, "cannot proto marshal %T", allowance)
}

a.Allowance, err = types.NewAnyWithValue(protoAllowance)
if err != nil {
return sdkerrors.Wrapf(sdkerrors.ErrPackAny, "cannot proto marshal %T", protoAllowance)
}
return nil
}

Expand Down
71 changes: 71 additions & 0 deletions x/feegrant/filtered_fee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"testing"
"time"

proto "github.com/gogo/protobuf/proto"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -188,3 +189,73 @@ func TestFilteredFeeValidAllow(t *testing.T) {
})
}
}

// invalidInterfaceAllowance does not implement proto.Message
type invalidInterfaceAllowance struct {
}

// compilation time interface implementation check
var _ feegrant.FeeAllowanceI = (*invalidInterfaceAllowance)(nil)

func (i invalidInterfaceAllowance) Accept(ctx sdk.Context, fee sdk.Coins, msgs []sdk.Msg) (remove bool, err error) {
return false, nil
}

func (i invalidInterfaceAllowance) ValidateBasic() error {
return nil
}

// invalidProtoAllowance can not run proto.Marshal
type invalidProtoAllowance struct {
invalidInterfaceAllowance
}

// compilation time interface implementation check
var _ feegrant.FeeAllowanceI = (*invalidProtoAllowance)(nil)
var _ proto.Message = (*invalidProtoAllowance)(nil)

func (i invalidProtoAllowance) Reset() {
}

func (i invalidProtoAllowance) String() string {
return ""
}

func (i invalidProtoAllowance) ProtoMessage() {
}

func TestSetAllowance(t *testing.T) {
cases := map[string]struct {
allowance feegrant.FeeAllowanceI
valid bool
}{
"valid allowance": {
allowance: &feegrant.BasicAllowance{},
valid: true,
},
"invalid interface allowance": {
allowance: &invalidInterfaceAllowance{},
valid: false,
},
"empty allowance": {
allowance: (*invalidProtoAllowance)(nil),
valid: false,
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
allowance := &feegrant.BasicAllowance{}
msgs := []string{sdk.MsgTypeURL(&banktypes.MsgSend{})}
allowed, err := feegrant.NewAllowedMsgAllowance(allowance, msgs)
require.NoError(t, err)
require.NotNil(t, allowed)
err = allowed.SetAllowance(tc.allowance)
if tc.valid {
require.NoError(t, err)
} else {
require.Error(t, err)
}
})
}
}

0 comments on commit 0ade7e4

Please sign in to comment.