Function BeforeTokenizeShareRecordRemoved does not work as expected
Description
In the RedeemTokensForShares function, when a user redeems all tokens, the delegation is deleted through unbonding, and the tokenized record is removed. Before the record is removed, the hook function BeforeTokenizeShareRecordRemoved is called, which triggers WithdrawSingleShareRecordReward to claim the reward.
// x/staking/keeper/msg_server.go
func (k msgServer) RedeemTokensForShares(goCtx context.Context, msg *types.MsgRedeemTokensForShares) (*types.MsgRedeemTokensForSharesResponse, error) {
// ...
returnAmount, err := k.Unbond(ctx, record.GetModuleAddress(), valAddr, shares)
// ...
// Note: since delegation object has been changed from unbond call, it gets latest delegation
_, err = k.GetDelegation(ctx, record.GetModuleAddress(), valAddr)
if err != nil && !errors.Is(err, types.ErrNoDelegation) {
return nil, err
}
// this err will be ErrNoDelegation
if err != nil {
if k.hooks != nil {
if err := k.hooks.BeforeTokenizeShareRecordRemoved(ctx, record.Id); err != nil {
return nil, err
}
}
err = k.DeleteTokenizeShareRecord(ctx, record.Id)
if err != nil {
return nil, err
}
}
// ...
// x/distribution/keeper/hooks.go
func (h Hooks) BeforeTokenizeShareRecordRemoved(ctx context.Context, recordID uint64) error {
err := h.k.WithdrawSingleShareRecordReward(ctx, recordID)
if err != nil {
h.k.Logger(ctx).Error(err.Error())
}
return err
}However, in the WithdrawSingleShareRecordReward function, it checks delegationFound, which is always false because the hook is called when ErrNoDelegation is encountered. As a result, the reward-claiming process is never executed through this path.
// x/distribution/keeper/keeper.go
func (k Keeper) WithdrawSingleShareRecordReward(ctx context.Context, recordID uint64) error {
// ...
delegationFound := true
_, err = k.stakingKeeper.Delegation(ctx, record.GetModuleAddress(), valAddr)
if err != nil {
if !goerrors.Is(err, stakingtypes.ErrNoDelegation) {
return err
}
delegationFound = false
}
sdkCtx := sdk.UnwrapSDKContext(ctx)
if validatorFound && delegationFound {
// withdraw rewards into reward module account and send it to reward owner
cacheCtx, write := sdkCtx.CacheContext()
_, err = k.WithdrawDelegationRewards(cacheCtx, record.GetModuleAddress(), valAddr)
// ...Impact
In current codebase, the Unbond function calls the BeforeDelegationSharesModified hook, and this function will claim rewards before removing the delegation. Nevertheless, the BeforeTokenizeShareRecordRemoved hook does not work as expected, and the WithdrawDelegationRewards is not reachable using this path.
Recommendations
Remove duplicate and unreachable code. If there is a possibility that the unbond hook does not exist, the call position should be adjusted to claim the rewards before the delegation is removed while redeeming all tokens.