-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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)
- Loading branch information
Showing
3 changed files
with
207 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) | ||
| } | ||
| }) | ||
| } | ||
| } |