Skip to content

fix(stakeweight): correct permanent lock epoch accounting#45

Merged
rplusq merged 2 commits into
mainfrom
fix/permanent-lock-bug-public-clean
May 12, 2026
Merged

fix(stakeweight): correct permanent lock epoch accounting#45
rplusq merged 2 commits into
mainfrom
fix/permanent-lock-bug-public-clean

Conversation

@rplusq
Copy link
Copy Markdown
Collaborator

@rplusq rplusq commented May 12, 2026

Summary

  • Reorders writes to userPermanentWeightAtEpoch in updatePermanentLock and increasePermanentLockDuration to occur after _checkpoint, so the parallel-array slot lands on the freshly-created user epoch instead of the previous one
  • Applies the same pre-checkpoint state-update pattern to convertToPermanent and triggerUnlock so intermediate epochs reflect correct permanent supply
  • Gates increaseLockAmount to redirect permanent-lock holders to updatePermanentLock; permits internal _increaseLockAmount callers to bypass the expired-lock check for permanent positions
  • Adds StakeWeightHealer, an admin-gated migration contract used in a sandwich-upgrade to backfill userPermanentWeightAtEpoch for affected users

Deployment status

The fix and healer migration were deployed and executed on Optimism mainnet on 2025-12-17 (0xa9b9...be51). This PR brings the canonical repository in line with what was shipped.

Test plan

  • CI passes
  • permanentLockFix.t.sol covers post-fix invariants for both updated functions
  • Existing fork tests in StakeWeightPermanentUpgradeFork.t.sol continue to pass

🤖 Generated with Claude Code

rplusq and others added 2 commits March 2, 2026 13:06
The OldPermanentLockStakeWeight.sol snapshot was retained only to support
the bug-demonstration test. The fix is deployed and verified on-chain, so
the demo carries no ongoing regression value — permanentLockFix.t.sol still
covers the post-fix invariants. History at 615f1a0 preserves the snapshot
if anyone needs it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude Bot commented May 12, 2026

Claude finished @rplusq's task in 3m 41s —— View job


PR Review: fix(stakeweight): correct permanent lock epoch accounting

  • Gather context
  • Read changed files
  • Spawn data classification subagent
  • Compile findings and post review

The epoch-ordering fix is correct. Pre-checkpoint state updates in convertToPermanent/triggerUnlock and post-checkpoint history writes in updatePermanentLock/increasePermanentLockDuration accurately solve the parallel-array off-by-one. The two-phase checkpoint in convertToPermanent correctly cancels slope changes. No data classification issues found.

Found 2 issue(s)

Issue 1: StakeWeightHealer missing _disableInitializers()

ID: stakeweighthealer-disable-initializers-a3f1
File: evm/src/StakeWeightHealer.sol:22
Severity: MEDIUM
Category: security

Context:

  • Pattern: StakeWeightHealer inherits AccessControlUpgradeable (which chains through Initializable) but has no constructor that calls _disableInitializers().
  • Risk: The bare implementation contract (not the proxy) can have its __AccessControl_init invoked, granting a malicious admin role on the implementation. While the proxy's storage is unaffected, the implementation itself is left exploitable.
  • Impact: An attacker who initializes the implementation could call batchHealPermanentWeights against the implementation's own (empty) storage slot, or use the initialized implementation as a stepping stone in more complex upgrade-slot attacks.
  • Trigger: Anyone can call OZ-inherited initializer functions directly on the deployed StakeWeightHealer implementation address while the contract is live between upgrade steps.

Recommendation:

/// @custom:oz-upgrades-unsafe-allow constructor
constructor() {
    _disableInitializers();
}

Issue 2: batchHealPermanentWeights heals only the latest user epoch, leaving intermediate epochs broken

ID: stakeweighthealer-partial-epoch-heal-7c2d
File: evm/src/StakeWeightHealer.sol:61
Severity: MEDIUM
Category: code_quality

Context:

  • Pattern: The healer checks only s.userPermanentWeightAtEpoch[user][userEpoch] == 0 (current epoch) and writes the fix there. Users who called updatePermanentLock or increasePermanentLockDuration multiple times under the buggy code will have N epochs all with userPermanentWeightAtEpoch == 0, but only epoch N is repaired.
  • Risk: balanceOfAt(user, blockNumber) and totalSupplyAt(blockNumber) for blocks that land in broken intermediate epochs (1 through N-1) continue to return zero for those users' permanent weight contribution.
  • Impact: Historical reward distributions (StakingRewardDistributor.balanceOfAt) computed for those past weeks remain incorrect — past claims based on those epochs could be under-calculated.
  • Trigger: Any user who invoked either function more than once between deployments.

Recommendation: Iterate backward through userPointHistory to heal all epochs with missing weight, not just the current one:

for (uint256 e = userEpoch; e >= 1; e--) {
    if (s.userPermanentWeightAtEpoch[user][e] != 0) break; // stop at first already-healed epoch
    s.userPermanentWeightAtEpoch[user][e] = currentWeight;
    emit PermanentWeightHealed(user, e, currentWeight);
}

(Since the healer is already deployed and the migration has run on mainnet, this is a post-mortem finding for completeness.)

@rplusq rplusq merged commit bcd9b88 into main May 12, 2026
7 of 8 checks passed
@rplusq rplusq deleted the fix/permanent-lock-bug-public-clean branch May 12, 2026 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant