How an Expired CurrentSui Reward Pool Could Refund Yield Borrowers Had Already Earned

An expired reward pool is not necessarily an economically empty reward pool.

That distinction caused this finding in CurrentSui’s borrow liquidity mining flow.

A borrower can already be active before a new reward campaign is created. Their participation is reflected in the global reward share accounting, but the borrower specific tracker for the new pool is created only when that borrower interacts again or claims.

If the campaign expires before that tracker is materialized, the close path can see:

num_obligation_reward_managers = 0

and treat the pool as safe to refund.

The problem is that zero materialized trackers does not mean zero economically accrued rewards.

The proof of concept showed both sides of that mismatch. If the borrower claims first, the borrower receives the reward and a later close refunds zero. If the pool is closed first, the close recipient receives the same value and the borrower can no longer claim it.

I reported the finding as High in the Sherlock CurrentSui contest. Sherlock ultimately classified it as Medium, and it earned $92.24.

Public Sherlock submission

The bug is therefore not simply about claim timing. It is about using lazy materialization as a proxy for reward ownership.

The accounting model creates economic rewards before claim

Claim is not the moment the reward first comes into existence.

CurrentSui tracks borrow liquidity mining participation globally through total_shares, and the pool later updates cumulative_rewards_per_share.

That means an active borrower can participate economically in a campaign even if the borrower specific reward tracker for that pool has not yet been created.

This matters because lazy accounting is only safe when every lifecycle function understands that some obligations may exist economically before they exist as explicit per user state.

The close path does not preserve that distinction.

A borrower can already be active before the campaign exists

When a user borrows, the borrow path updates the obligation reward manager immediately:

let miner = market.borrow_liquidity_mining_mut<MarketType>();

miner.update_obligation_reward_manager<MarketType, CoinType>(
    get_borrow_reward_type(),
    obligation_owner_cap.id(),
    total_borrow.floor(),
    clock
);

Source: borrow.move

So a borrower can already have active borrow reward shares when a later reward pool is created.

That is the setup used by the PoC: the borrower exists first, then the campaign is added.

A new pool starts with no materialized borrower trackers

A newly created reward pool starts with:

num_obligation_reward_managers: 0

along with zero allocated rewards and zero cumulative rewards per share:

let pool_reward = PoolReward {
    id,
    reward_type: object::id::<TypeName>(&RewardType<CoinType> {}),
    start_time_ms,
    end_time_ms,
    total_rewards: rewards.value(),
    allocated_rewards: float::from(0),
    cumulative_rewards_per_share: float::from(0),
    num_obligation_reward_managers: 0,
    additional_fields
};

Source: reward_manager.move

Starting at zero is not itself a problem.

The problem appears later when the close path interprets that counter as evidence that no borrower reward remains.

Rewards accrue through the global share total

The pool update logic distributes unlocked rewards across total_shares:

if (pool_reward_manager.total_shares == 0) {
    pool_reward_manager.last_update_time_ms = cur_time_ms;
    return
};

let unlocked_rewards =
    float::from(pool_reward.total_rewards).mul(
        float::from(time_passed_ms)
    ).div(
        float::from(pool_reward.end_time_ms - pool_reward.start_time_ms)
    );

pool_reward.allocated_rewards =
    pool_reward.allocated_rewards.add(unlocked_rewards);

pool_reward.cumulative_rewards_per_share =
    pool_reward.cumulative_rewards_per_share.add(
        unlocked_rewards.div(
            float::from(pool_reward_manager.total_shares)
        )
    );

Source: reward_manager.move

As time passes, value becomes attributable to active shares.

So the system can already have borrower yield that is economically earned even though the pool still has no materialized borrower tracker for that user.

The borrower tracker is created lazily

The user specific tracker is filled later inside update_obligation_reward_manager.

That same path increments num_obligation_reward_managers:

let optional_reward = obligation_reward_manager.rewards.borrow_mut(i);

if (optional_reward.is_none()) {
    if (obligation_reward_manager.last_update_time_ms <= pool_reward.end_time_ms) {
        optional_reward.fill(ObligationReward {
            pool_reward_id: object::id(pool_reward),
            earned_rewards: {
                if (obligation_reward_manager.last_update_time_ms <= pool_reward.start_time_ms) {
                    pool_reward.cumulative_rewards_per_share.mul(
                        float::from(obligation_reward_manager.share)
                    )
                } else {
                    float::from(0)
                }
            },
            cumulative_rewards_per_share: pool_reward.cumulative_rewards_per_share
        });

        pool_reward.num_obligation_reward_managers =
            pool_reward.num_obligation_reward_managers + 1;
    };
}

Source: reward_manager.move

This creates the dangerous state:

borrower has active economic participation
YES

borrower specific tracker for this pool
NO

num_obligation_reward_managers
0

The counter describes materialization state, not necessarily economic obligation state.

The expired pool close path relies on that counter

The close function extracts the pool, checks that the campaign has ended, and then requires:

assert!(
    num_obligation_reward_managers == 0,
    error::liquidity_mining_not_all_rewards_claimed()
);

Relevant code:

public(package) fun close_pool_reward<CoinType>(
    pool_reward_manager: &mut PoolRewardManager,
    index: u64,
    clock: &Clock
): Balance<CoinType> {
    let optional_pool_reward =
        pool_reward_manager.pool_rewards.borrow_mut(index);

    let PoolReward {
        id,
        reward_type: _,
        start_time_ms: _,
        end_time_ms,
        total_rewards: _,
        allocated_rewards: _,
        cumulative_rewards_per_share: _,
        num_obligation_reward_managers,
        mut additional_fields,
    } = optional_pool_reward.extract();

    object::delete(id);

    let cur_time_ms = clock.timestamp_ms();

    assert!(
        cur_time_ms >= end_time_ms,
        error::liquidity_mining_pool_reward_period_not_over()
    );

    assert!(
        num_obligation_reward_managers == 0,
        error::liquidity_mining_not_all_rewards_claimed()
    );

    let reward_balance =
        additional_fields.remove<
            RewardBalance<CoinType>,
            Balance<CoinType>
        >(RewardBalance<CoinType>{});

Source: reward_manager.move

The close path does not first refresh the pool and materialize the reward state of already active borrowers.

As a result, it can interpret a zero tracker count as permission to refund a pool whose global share accounting would produce a nonzero borrower claim.

The claim path exposes the mismatch

The normal claim path does something the close path does not.

Before reading the borrower’s reward tracker, it calls:

update_obligation_reward_manager(
    pool_reward_manager,
    obligation_id,
    clock,
    false
);

Source: reward_manager.move

That update materializes the borrower’s reward state.

So the same expired campaign can produce two mutually exclusive ownership outcomes:

Borrower claims first
→ reward state is materialized
→ borrower receives the accrued reward
→ later close has nothing to refund

or:

Pool closes first
→ tracker count is still zero
→ reward balance is refunded
→ borrower cannot realize the reward afterward

The borrower’s economic participation is the same in both paths. Only the timing of materialization changes.

The PoC proves the value is the borrower’s reward, not leftover dust

The Move PoC uses five tests that answer different questions.

Test	What it proves
ctl	An already active borrower can claim more than 10 USDC after the campaign expires.
tail	If the borrower claims first, the later pool close refund is exactly zero.
exp	If the pool closes first, the close recipient receives more than 10 USDC.
cmp	The normal borrower claim is exactly equal to the close first refund.
dead	After the pool closes first, the borrower can no longer claim the reward.

The strongest comparison is in cmp:

assert!(a > TEN, 5);
assert!(b > TEN, 6);
assert!(t == 0, 7);
assert!(a + t == REWARD, 8);
assert!(a == b, 9);

Here:

a = borrower claim in the normal path
t = refund after the normal claim
b = refund when the pool closes first

The assertions prove:

t = 0
a = b
a + t = REWARD

So the close first path is not collecting unrelated campaign dust.

It receives the same value that the borrower receives in the normal path.

The PoC defines REWARD as 1,000 USDC, and the comparison test accounts for that entire amount through the control claim and zero tail refund.

The full Move test suite passes

The PoC lives in:

tests/integration/test_cases/poc_close.move

and runs with:

sui move test protocol::poc_close -i 100000000000

The reported result was:

[ PASS    ] protocol::poc_close::cmp
[ PASS    ] protocol::poc_close::ctl
[ PASS    ] protocol::poc_close::dead
[ PASS    ] protocol::poc_close::exp
[ PASS    ] protocol::poc_close::tail

Test result: OK.
Total tests: 5; passed: 5; failed: 0

Together, these tests establish the full lifecycle problem: the borrower can claim the reward normally, the post claim refund is zero, closing first redirects the same value, and the borrower cannot recover it afterward.

Impact and severity context

The demonstrated impact is direct loss of borrower yield.

The finding does not depend on malicious privileged behavior. Closing an expired pool is part of the intended reward lifecycle, and there is no contract rule requiring every already active borrower to claim before closure.

The problem is that the close function itself treats:

num_obligation_reward_managers == 0

as its safety condition even though that counter only reflects materialized trackers.

My original High assessment was based on the fact that the PoC demonstrates user yield loss greater than 10 USDC and shows that the same value moves to the close recipient instead.

The official Sherlock classification is Medium, which is the severity I use in my public record.

Regardless of severity, the technical failure is clear: pool closure can refund value that the normal claim path proves was economically attributable to an eligible borrower.

Fix the lifecycle, not only the counter

The core invariant should be:

Closing a reward campaign must not destroy or refund rewards that have already accrued economically to eligible users.

There are several possible directions.

The pool can be fully refreshed before refund logic runs.

Existing obligations can have their reward trackers materialized when a new pool is created.

A cleaner long term design is to separate campaign finalization from reward ownership: closing the campaign stops new accrual, while rewards already earned before expiry remain claimable.

Only value that is provably not attributable to eligible users should become refundable.

A regression test should preserve the control and close first comparison from the PoC. If a borrower can claim X immediately before closure, closing the campaign must not make that same X refundable to another recipient.

The broader lesson

Lazy accounting is useful because it avoids updating every user whenever global state changes.

But it creates a strict design requirement:

not materialized
does not mean
not economically owed

Any close, sweep, refund, cleanup, or migration path that treats lazy state as complete state risks erasing obligations that would have appeared on the next user interaction.

Reward systems are especially sensitive because time can create economic entitlement while user specific state remains untouched.

The safest review question is simple:

If a user can claim X immediately before closure,
can closure make X disappear or become refundable elsewhere?

If the answer is yes, the lifecycle is not preserving reward ownership.

Conclusion

The CurrentSui reward pool close path used materialized borrower trackers as a proxy for outstanding economic obligations.

That proxy was incomplete.

An already active borrower could accrue rewards through the global share accounting while the pool still reported zero materialized obligation reward managers. If the borrower interacted first, the normal claim path materialized the reward and paid it to the borrower. If the expired pool closed first, the same value could be refunded and the borrower could no longer claim it.

The core issue is not reward calculation accuracy. It is reward ownership being decided by the timing of lazy materialization.

The engineering rule is broader than this one protocol:

Lifecycle functions must reason about economic obligations, not only the objects that have already been materialized to represent them.