Assessment reports>Cosmos SDK Liquid Stake Module>Low findings>Function ,BeforeTokenizeShareRecordRemoved, does not work as expected
Category: Business Logic

Function BeforeTokenizeShareRecordRemoved does not work as expected

Low Severity
Low Impact
High Likelihood

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.

Remediation

Zellic © 2025Back to top ↑