fix: fix bug when updating allowance inside AllowedMsgAllowance (#10564)
## Description Closes: #10563 By the fix, AllowedMsgAllowance updates its allowance after the change in `Accept()`. To catch the corresponding issue, the fix also adds new unit tests. --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [x] added `!` to the type prefix if API or client breaking change - [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [x] provided a link to the relevant issue or specification - [x] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [x] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [x] added a changelog entry to `CHANGELOG.md` - [x] included comments for [documenting Go code](https://blog.golang.org/godoc) - [x] updated the relevant documentation or specification - [x] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable)
This commit is contained in:
parent
e9741334df
commit
651ef9ea6e
|
@ -190,6 +190,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
|
||||||
|
|
||||||
### State Machine Breaking
|
### 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`.
|
* [\#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) [#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.
|
* (store) [#10218](https://github.com/cosmos/cosmos-sdk/pull/10218) Charge gas even when there are no entries while seeking.
|
||||||
|
|
|
@ -50,6 +50,17 @@ func (a *AllowedMsgAllowance) GetAllowance() (FeeAllowanceI, error) {
|
||||||
return allowance, nil
|
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
|
// 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) {
|
func (a *AllowedMsgAllowance) Accept(ctx sdk.Context, fee sdk.Coins, msgs []sdk.Msg) (bool, error) {
|
||||||
if !a.allMsgTypesAllowed(ctx, msgs) {
|
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 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 {
|
func (a *AllowedMsgAllowance) allowedMsgsToMap(ctx sdk.Context) map[string]bool {
|
||||||
|
|
|
@ -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)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
Loading…
Reference in New Issue