farming-pool: no test proves set_global_multiplier has zero effect on the lock/position accrual path — the boost-only-applies-to-stake invariant is unverified

[prodbycorne]

Overview

farming-pool has two structurally parallel but functionally independent accrual systems: the "Boost / Stake" system (stake/unstake/set_boost, backed by UserStake, whose checkpoint/compute_credits formula factors in global_multiplier) and the "lock" system (lock_assets/unlock_assets, backed by Position, whose checkpoint_position/calculate_credits formula never references global_multiplier at all):

// stake path — factors in global_multiplier
fn checkpoint(env: &Env, user: &Address, stake: &mut UserStake) {
    let allocation_pct = get_user_boost(env, user).unwrap_or(0);
    let multiplier = read_global_multiplier(env);          // <-- live multiplier read
    ...
    stake.credits_banked += compute_credits(stake.amount, allocation_pct, multiplier, stake.credit_rate, elapsed);
    ...
}

// lock path — never touches global_multiplier at all
fn checkpoint_position(env: &Env, position: &mut Position) {
    let elapsed = current.saturating_sub(position.checkpoint_ledger);
    position.total_credits += position.amount * position.credit_rate * elapsed as i128;
    ...
}

This is a real, load-bearing design invariant — locked positions are never boosted, only staked positions are — but it has no dedicated test asserting it holds. #60/#59 (closed) already established, and tested, that changing global_multiplier mid-accrual does retroactively reprice a staker's uncheckpointed window (the stake-side behavior). There is no equivalent test proving the opposite is true for the lock side: that a set_global_multiplier call has zero effect on any locked position's accrued or accruing credits, ever, regardless of when it's called relative to a lock's lifecycle.

Every existing multiplier-related test (test_admin_sets_global_multiplier, test_admin_multiplier_change_applies_from_next_checkpoint, test_admin_multiplier_rejects_zero, test_set_global_multiplier_rejects_above_ceiling, etc. — farming-pool/src/test.rs:404-660) exercises the stake system exclusively; none of them ever call lock_assets/unlock_assets/calculate_credits in combination with a set_global_multiplier call, so this cross-system independence — which today happens to hold by construction (the lock formula simply never reads the multiplier), but is only one accidental refactor away from silently breaking (e.g. if checkpoint_position is ever consolidated with checkpoint/compute_credits per #63's open "duplicate formula" concern, and that consolidation mistakenly threads global_multiplier through to the lock path too) — has no regression test that would catch such a regression.

Requirements

• Add a test that explicitly constructs a scenario where a user has an active Position (locked, unboosted), changes global_multiplier via set_global_multiplier mid-lock, advances ledgers, and asserts the position's accrued credits are computed exactly as if the multiplier change never happened — i.e. purely amount * credit_rate * elapsed, with no trace of the multiplier anywhere in the result.
• Ideally combine this with a parallel UserStake for the same user in the same test, so the test directly demonstrates the divergence: the stake side's credits do reflect the new multiplier from the next checkpoint (per #60's already-established, intentional behavior) while the lock side's credits, computed over the identical elapsed window, do not.

Acceptance Criteria

• A new test proves a set_global_multiplier change has zero measurable effect on a concurrently-open Position's accrued/accruing credits.
• The test explicitly documents (via a doc comment on the test itself) that this is asserting a design invariant, not incidental behavior, so a future refactor that breaks it fails loudly and legibly rather than needing to be independently rediscovered.
• If feasible, the same test (or a paired one) directly contrasts this with the stake-side's documented retroactive-repricing behavior from #60, for maximum clarity that the divergence is intentional and being actively guarded, not an oversight the test suite happens not to have noticed yet.

Additional Notes

More precise references

• soroban/contracts/farming-pool/src/lib.rs:265-279 (checkpoint, stake path) vs. :281-287 (checkpoint_position, lock path) — confirmed via grep -n "global_multiplier\|read_global_multiplier" soroban/contracts/farming-pool/src/lib.rs that read_global_multiplier/global_multiplier appear only in stake-path functions (checkpoint, get_credits, get_boost_config, the setters themselves) and never in checkpoint_position, calculate_credits, or anywhere else touching Position.
• farming-pool/src/test.rs:404-660 — the full block of multiplier-related tests, confirmed all constructed exclusively around UserStake/stake()/get_credits(), never Position/lock_assets()/calculate_credits().
• farming-pool/src/test.rs:539-552 (test_credit_rate_change_does_not_retroactively_alter_locked_credits) — the closest existing analog, but this tests credit_rate (which the lock path does use, via its own position.credit_rate snapshot) not global_multiplier (which the lock path never uses at all) — confirming this issue's gap is specifically about the multiplier, not a general absence of lock-path retroactivity testing.
• #63 (open, "checkpoint_position and calculate_credits duplicate the credit-accrual formula instead of sharing one implementation") — the specific future-refactor risk this issue's test would guard against, since a naive consolidation of the two formulas is exactly the kind of change that could accidentally thread global_multiplier into the lock path.

Additional edge cases

• Worth also asserting the inverse framing explicitly: that get_boost_config/set_boost/UserBoost (the allocation-percentage mechanism) have no effect on Position either — a user could call set_boost for themselves while also holding a Position, and the test should confirm the Position's credits are entirely unaffected by that unrelated stake-system configuration, reinforcing that the two systems are genuinely independent, not just "multiplier happens not to apply" as an isolated fact.

Test/reproduction plan

• test_global_multiplier_change_does_not_affect_locked_position_credits: setup(1, 1); lock_assets(user, 1000); advance_ledgers(env, 10); set_global_multiplier(5) (a large change); advance_ledgers(env, 10); assert calculate_credits(user) == 1000 * 1 * 20 (i.e. credit_rate=1 applied uniformly across the full 20 elapsed ledgers, with zero trace of the multiplier having ever changed).
• Extended variant with a parallel stake(user, 1000) in the same test, asserting the stake side's get_credits(user) does reflect the multiplier change for its own uncheckpointed window (per #60), directly contrasting the two systems' behavior in one test for clarity.

Cross-references

• farming-pool: get_credits retroactively reprices a user's entire uncheckpointed accrual window when the admin changes the global multiplier #59/farming-pool: get_credits retroactively reprices a user's entire uncheckpointed accrual window when the admin changes the global multiplier #60 (closed) — establish and test the stake-side retroactive-repricing behavior this issue's test explicitly contrasts against.
• farming-pool: checkpoint_position and calculate_credits duplicate the credit-accrual formula instead of sharing one implementation #63 (open) — the future-refactor risk (formula consolidation) this issue's test is specifically designed to catch a regression from.

[Chigybillionz]

Hi, I’ve reviewed this issue and I’m confident I can fix it. I’d like to take it on.

[grantfox-oss]

🦊 GrantFox — @Chigybillionz has been assigned to this issue as part of the Third Campaign campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #130)
2. Your PR will be reviewed by the SmartDropLabs maintainers

Good luck! Track your progress on GrantFox.