-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
chore: (x/feegrant) update keeper logic to do not allow duplicate grant #14294
Changes from 1 commit
fcc5eb5
6662169
c39b6b4
1a26624
93e2b22
dd5a28e
1a1e605
c9c06e7
89a30d9
181f4fc
c806587
6481d32
68f3094
53199e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,11 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger { | |
|
||
// GrantAllowance creates a new grant | ||
func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress, feeAllowance feegrant.FeeAllowanceI) error { | ||
// Checking for duplicate entry | ||
if f, _ := k.GetAllowance(ctx, granter, grantee); f != nil { | ||
amaury1093 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "fee allowance already exists") | ||
} | ||
|
||
// create the account if it is not in account state | ||
granteeAcc := k.authKeeper.GetAccount(ctx, grantee) | ||
if granteeAcc == nil { | ||
|
@@ -51,54 +56,21 @@ func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress, | |
store := ctx.KVStore(k.storeKey) | ||
key := feegrant.FeeAllowanceKey(granter, grantee) | ||
|
||
var oldExp *time.Time | ||
existingGrant, err := k.getGrant(ctx, granter, grantee) | ||
|
||
// If we didn't find any grant, we don't return any error. | ||
// All other kinds of errors are returned. | ||
if err != nil && !sdkerrors.IsOf(err, sdkerrors.ErrNotFound) { | ||
exp, err := feeAllowance.ExpiresAt() | ||
if err != nil { | ||
Comment on lines
+59
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought the PR was only about moving the duplicate entry check from msg_server to keeper. Can you explain what these changes are about? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. up There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the removed logic is for handling duplicate grant (previously we are updating the expiration with the newer grant's expiration for the duplicate grant), which is now unreachable if duplicate grant found. |
||
return err | ||
} | ||
|
||
if existingGrant != nil && existingGrant.GetAllowance() != nil { | ||
grantInfo, err := existingGrant.GetGrant() | ||
if err != nil { | ||
return err | ||
} | ||
|
||
oldExp, err = grantInfo.ExpiresAt() | ||
if err != nil { | ||
return err | ||
} | ||
} | ||
|
||
newExp, err := feeAllowance.ExpiresAt() | ||
switch { | ||
case err != nil: | ||
return err | ||
|
||
case newExp != nil && newExp.Before(ctx.BlockTime()): | ||
// expiration shouldn't be in the past. | ||
if exp != nil && exp.Before(ctx.BlockTime()) { | ||
return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "expiration is before current block time") | ||
} | ||
|
||
case oldExp == nil && newExp != nil: | ||
// when old oldExp is nil there won't be any key added before to queue. | ||
// add the new key to queue directly. | ||
k.addToFeeAllowanceQueue(ctx, key[1:], newExp) | ||
|
||
case oldExp != nil && newExp == nil: | ||
// when newExp is nil no need of adding the key to the pruning queue | ||
// remove the old key from queue. | ||
k.removeFromGrantQueue(ctx, oldExp, key[1:]) | ||
|
||
case oldExp != nil && newExp != nil && !oldExp.Equal(*newExp): | ||
// if expiry is not nil, add the new key to pruning queue. | ||
if exp != nil { | ||
// `key` formed here with the prefix of `FeeAllowanceKeyPrefix` (which is `0x00`) | ||
// remove the 1st byte and reuse the remaining key as it is. | ||
|
||
// remove the old key from queue. | ||
k.removeFromGrantQueue(ctx, oldExp, key[1:]) | ||
|
||
// add the new key to queue. | ||
k.addToFeeAllowanceQueue(ctx, key[1:], newExp) | ||
// remove the 1st byte and reuse the remaining key as it is | ||
k.addToFeeAllowanceQueue(ctx, key[1:], exp) | ||
} | ||
|
||
grant, err := feegrant.NewGrant(granter, grantee, feeAllowance) | ||
|
@@ -312,12 +284,6 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) (*feegrant.GenesisState, error) { | |
}, err | ||
} | ||
|
||
func (k Keeper) removeFromGrantQueue(ctx sdk.Context, exp *time.Time, allowanceKey []byte) { | ||
key := feegrant.FeeAllowancePrefixQueue(exp, allowanceKey) | ||
store := ctx.KVStore(k.storeKey) | ||
store.Delete(key) | ||
} | ||
|
||
func (k Keeper) addToFeeAllowanceQueue(ctx sdk.Context, grantKey []byte, exp *time.Time) { | ||
store := ctx.KVStore(k.storeKey) | ||
store.Set(feegrant.FeeAllowancePrefixQueue(exp, grantKey), []byte{}) | ||
|
Check warning
Code scanning / gosec
Returned error is not propagated up the stack.