fix: fix bug when updating allowance inside AllowedMsgAllowance

CHANGELOG.md

@@ -188,6 +188,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### State Machine Breaking
 
+* [\#10564](https://github.com/cosmos/cosmos-sdk/pull/10564) Fix bug when updating allowance inside AllowedMsgAllowance
 * [\#10536](https://github.com/cosmos/cosmos-sdk/pull/10536]) Enable `SetSequence` for `ModuleAccount`.
 * (store) [#10247](https://github.com/cosmos/cosmos-sdk/pull/10247) Charge gas for the key length in gas meter.
 * (store) [#10218](https://github.com/cosmos/cosmos-sdk/pull/10218) Charge gas even when there are no entries while seeking.

x/feegrant/filtered_fee.go

@@ -50,6 +50,17 @@ func (a *AllowedMsgAllowance) GetAllowance() (FeeAllowanceI, error) {
 	return allowance, nil
 }
 
+// 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 {
+		return sdkerrors.Wrapf(sdkerrors.ErrPackAny, "cannot proto marshal %T", allowance)
+	}
+
+	return nil
+}
+
 // Accept method checks for the filtered messages has valid expiry
 func (a *AllowedMsgAllowance) Accept(ctx sdk.Context, fee sdk.Coins, msgs []sdk.Msg) (bool, error) {
 	if !a.allMsgTypesAllowed(ctx, msgs) {
@@ -61,7 +72,13 @@ func (a *AllowedMsgAllowance) Accept(ctx sdk.Context, fee sdk.Coins, msgs []sdk.
 		return false, err
 	}
 
-	return allowance.Accept(ctx, fee, msgs)
+	remove, err := allowance.Accept(ctx, fee, msgs)
+	if err == nil && !remove {
+		if err = a.SetAllowance(allowance); err != nil {
+			return false, err
+		}
+	}
+	return remove, err
 }
 
 func (a *AllowedMsgAllowance) allowedMsgsToMap(ctx sdk.Context) map[string]bool {

x/feegrant/filtered_fee_test.go

@@ -0,0 +1,188 @@
+package feegrant_test
+
+import (
+	"testing"
+	"time"
+
+	"github.com/cosmos/cosmos-sdk/x/feegrant"
+	ocproto "github.com/tendermint/tendermint/proto/tendermint/types"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/cosmos/cosmos-sdk/simapp"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
+)
+
+func TestFilteredFeeValidAllow(t *testing.T) {
+	app := simapp.Setup(t, false)
+
+	ctx := app.BaseApp.NewContext(false, ocproto.Header{
+		Time: time.Now(),
+	})
+	eth := sdk.NewCoins(sdk.NewInt64Coin("eth", 10))
+	atom := sdk.NewCoins(sdk.NewInt64Coin("atom", 555))
+	smallAtom := sdk.NewCoins(sdk.NewInt64Coin("atom", 43))
+	bigAtom := sdk.NewCoins(sdk.NewInt64Coin("atom", 1000))
+	leftAtom := sdk.NewCoins(sdk.NewInt64Coin("atom", 512))
+	now := ctx.BlockTime()
+	oneHour := now.Add(1 * time.Hour)
+
+	// msg we will call in the all cases
+	call := banktypes.MsgSend{}
+	cases := map[string]struct {
+		allowance *feegrant.BasicAllowance
+		msgs      []string
+		fee       sdk.Coins
+		blockTime time.Time
+		accept    bool
+		remove    bool
+		remains   sdk.Coins
+	}{
+		"msg contained": {
+			allowance: &feegrant.BasicAllowance{},
+			msgs:      []string{sdk.MsgTypeURL(&call)},
+			accept:    true,
+		},
+		"msg not contained": {
+			allowance: &feegrant.BasicAllowance{},
+			msgs:      []string{"/cosmos.gov.v1beta1.MsgVote"},
+			accept:    false,
+		},
+		"small fee without expire": {
+			allowance: &feegrant.BasicAllowance{
+				SpendLimit: atom,
+			},
+			msgs:    []string{sdk.MsgTypeURL(&call)},
+			fee:     smallAtom,
+			accept:  true,
+			remove:  false,
+			remains: leftAtom,
+		},
+		"all fee without expire": {
+			allowance: &feegrant.BasicAllowance{
+				SpendLimit: smallAtom,
+			},
+			msgs:   []string{sdk.MsgTypeURL(&call)},
+			fee:    smallAtom,
+			accept: true,
+			remove: true,
+		},
+		"wrong fee": {
+			allowance: &feegrant.BasicAllowance{
+				SpendLimit: smallAtom,
+			},
+			msgs:   []string{sdk.MsgTypeURL(&call)},
+			fee:    eth,
+			accept: false,
+		},
+		"non-expired": {
+			allowance: &feegrant.BasicAllowance{
+				SpendLimit: atom,
+				Expiration: &oneHour,
+			},
+			msgs:      []string{sdk.MsgTypeURL(&call)},
+			fee:       smallAtom,
+			blockTime: now,
+			accept:    true,
+			remove:    false,
+			remains:   leftAtom,
+		},
+		"expired": {
+			allowance: &feegrant.BasicAllowance{
+				SpendLimit: atom,
+				Expiration: &now,
+			},
+			msgs:      []string{sdk.MsgTypeURL(&call)},
+			fee:       smallAtom,
+			blockTime: oneHour,
+			accept:    false,
+			remove:    true,
+		},
+		"fee more than allowed": {
+			allowance: &feegrant.BasicAllowance{
+				SpendLimit: atom,
+				Expiration: &oneHour,
+			},
+			msgs:      []string{sdk.MsgTypeURL(&call)},
+			fee:       bigAtom,
+			blockTime: now,
+			accept:    false,
+		},
+		"with out spend limit": {
+			allowance: &feegrant.BasicAllowance{
+				Expiration: &oneHour,
+			},
+			msgs:      []string{sdk.MsgTypeURL(&call)},
+			fee:       bigAtom,
+			blockTime: now,
+			accept:    true,
+		},
+		"expired no spend limit": {
+			allowance: &feegrant.BasicAllowance{
+				Expiration: &now,
+			},
+			msgs:      []string{sdk.MsgTypeURL(&call)},
+			fee:       bigAtom,
+			blockTime: oneHour,
+			accept:    false,
+		},
+	}
+
+	for name, stc := range cases {
+		tc := stc // to make scopelint happy
+		t.Run(name, func(t *testing.T) {
+			err := tc.allowance.ValidateBasic()
+			require.NoError(t, err)
+
+			ctx := app.BaseApp.NewContext(false, ocproto.Header{}).WithBlockTime(tc.blockTime)
+
+			// create grant
+			var granter, grantee sdk.AccAddress
+			allowance, err := feegrant.NewAllowedMsgAllowance(tc.allowance, tc.msgs)
+			require.NoError(t, err)
+			grant, err := feegrant.NewGrant(granter, grantee, allowance)
+			require.NoError(t, err)
+
+			// now try to deduct
+			removed, err := allowance.Accept(ctx, tc.fee, []sdk.Msg{&call})
+			if !tc.accept {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+
+			require.Equal(t, tc.remove, removed)
+			if !removed {
+				// mimic save & load process (#10564)
+				// the cached allowance was correct even before the fix,
+				// however, the saved value was not.
+				// so we need this to catch the bug.
+
+				// create a new updated grant
+				newGrant, err := feegrant.NewGrant(
+					sdk.AccAddress(grant.Granter),
+					sdk.AccAddress(grant.Grantee),
+					allowance)
+				require.NoError(t, err)
+
+				// save the grant
+				cdc := simapp.MakeTestEncodingConfig().Codec
+				bz, err := cdc.Marshal(&newGrant)
+				require.NoError(t, err)
+
+				// load the grant
+				var loadedGrant feegrant.Grant
+				err = cdc.Unmarshal(bz, &loadedGrant)
+				require.NoError(t, err)
+
+				newAllowance, err := loadedGrant.GetGrant()
+				require.NoError(t, err)
+				feeAllowance, err := newAllowance.(*feegrant.AllowedMsgAllowance).GetAllowance()
+				require.NoError(t, err)
+				assert.Equal(t, tc.remains, feeAllowance.(*feegrant.BasicAllowance).SpendLimit)
+			}
+		})
+	}
+}