velike: Remove Lock of reward, allow new reward contract to deploy and partial withdraw#307
velike: Remove Lock of reward, allow new reward contract to deploy and partial withdraw#307rickmak merged 19 commits intolikecoin:mainfrom
Conversation
…ove period lock - Change IRewardContract.withdraw(address) to withdraw(address, uint256) - Remove _isActive() check from veLikeReward.withdraw() so users can withdraw anytime - Support partial withdraw by subtracting the specified amount instead of zeroing - Update veLike._withdraw() to pass assets amount to reward contract
- Add isLegacyRewardContract mapping to veLikeStorage for allowlisting old reward contracts - Add setLegacyRewardContract(address, bool) owner function to manage the allowlist - Add claimLegacyReward(address) user-callable function to claim from rotated-out reward contracts - Add ErrNotLegacyRewardContract error for unauthorized legacy reward claims
- Remove setLockTime from test factory (no-lock operational model) - Replace 'lockable vault' test section with 'no-lock vault' tests - Add test: withdraw during active reward period succeeds - Add test: partial withdraw keeps remaining stake - Add test: reward tracking after partial withdraw is correct - Preserve existing post-period withdraw and claim tests
- Test setLegacyRewardContract owner-only access control - Test claimLegacyReward reverts on non-allowlisted contract - Test claimLegacyReward succeeds after allowlisting and rotation - Test claimLegacyReward reverts after removing from allowlist
Copy veLikeReward.sol as veLikeRewardNoLock.sol with only the contract name renamed. This serves as the base for the no-lock reward contract changes in the next commit.
|
"auto enrol" means what |
|
enroll* , it means no user action is needed to entitle the new reward assigned to veLike. |
There was a problem hiding this comment.
Pull request overview
This PR updates the veLike staking/reward flow to support a no-lock vault model, partial withdrawals, reward contract rotation to a new veLikeRewardNoLock implementation, and user/admin claiming of rewards from rotated-out (legacy) reward contracts.
Changes:
- Move lock enforcement to
veLike(vialockTime) and enable partial withdrawals by passingamountthrough to reward contracts. - Add
veLikeRewardNoLockreward contract + Ignition module + test fixtures and integration tests for rotation/auto-enrollment. - Add legacy reward allowlisting +
claimLegacyRewardentrypoint and a Hardhat task to claim legacy rewards.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| likecoin3/test/veLike.ts | Adds extensive test coverage for no-lock withdrawals, partial withdraw reward math, reward rotation/auto-enrollment, and legacy reward claiming. |
| likecoin3/test/factory.ts | Adjusts fixtures to default to no-lock behavior and adds NoLock-specific deployment/condition helpers. |
| likecoin3/tasks/staking.ts | Adds a Hardhat task for calling veLike.claimLegacyReward(...) in ops flows. |
| likecoin3/ignition/modules/veLikeRewardNoLock.ts | Adds Ignition module to deploy/configure veLikeRewardNoLock behind an ERC1967Proxy and initialize totalStaked. |
| likecoin3/contracts/veLikeRewardNoLock.sol | Introduces the new reward implementation supporting lazy staker sync (auto-enrollment) after rotation. |
| likecoin3/contracts/veLikeReward.sol | Updates reward withdraw handling to accept an amount (enabling partial withdraw) and removes withdraw-time active-period locking. |
| likecoin3/contracts/veLike.sol | Updates reward interface to pass withdraw amounts, moves lock enforcement into vault withdraw, and adds legacy reward allowlist + claim entrypoint. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * correct denominator that includes all existing vault holders. | ||
| */ | ||
| function initTotalStaked() external onlyOwner { | ||
| veLikeRewardStorage storage $ = _getveLikeRewardData(); |
There was a problem hiding this comment.
initTotalStaked() can be called repeatedly and will overwrite totalStaked based on the current vault totalSupply(). If this is called after deposits/withdrawals have already been tracked in stakerInfos, it will desync accounting and can break reward distribution. Consider guarding it so it can only be executed once (e.g., require !autoSyncEnabled) and/or only before any staking condition has started.
| veLikeRewardStorage storage $ = _getveLikeRewardData(); | |
| veLikeRewardStorage storage $ = _getveLikeRewardData(); | |
| // Prevent re-initialization, which would desync accounting once staking is live. | |
| require(!$.autoSyncEnabled, "Already initialized"); |
likecoin3/contracts/veLikeReward.sol
Outdated
| function withdraw( | ||
| address account, | ||
| uint256 amount | ||
| ) public whenNotPaused onlyVault { | ||
| veLikeRewardStorage storage $ = _getveLikeRewardData(); | ||
| if (_isActive()) { | ||
| revert ErrWithdrawLocked(); | ||
| } | ||
| _updateVault(); | ||
| _claimReward(account, false); | ||
| $.totalStaked -= $.stakerInfos[account].stakedAmount; | ||
| $.stakerInfos[account].stakedAmount = 0; | ||
| $.totalStaked -= amount; | ||
| $.stakerInfos[account].stakedAmount -= amount; | ||
| } |
There was a problem hiding this comment.
After removing the withdraw-time lock from withdraw(), the ErrWithdrawLocked custom error declared in this contract is no longer referenced anywhere in veLikeReward.sol. Consider removing it to avoid confusion about where the lock is enforced (it now appears to be enforced in veLike).
- Remove ErrWithdrawLocked dead code - Add autoSyncEnabled flag to storage for rotation safety - Add _effectiveStakedAmount() for vault-balance fallback on un-synced stakers - Add _syncStaker() for lazy enrollment of pre-rotation stakers - Add initTotalStaked() to set denominator from vault totalSupply - Guard getPendingReward against division by zero (totalStaked==0, endTime<=startTime) - Call _syncStaker in deposit, withdraw, and claimReward paths
Move rewardContract.deposit() before _mint() so that _syncStaker reads the pre-deposit balanceOf for correct retroactive reward math. Without this, vault.balanceOf(account) inside _syncStaker would include the new deposit, causing incorrect stakedAmount initialization.
…lment Add deployVeLikeRewardNoLock, initialMintNoLock, and initialConditionNoLock fixtures in factory.ts. Update veLike.ts to use NoLock fixtures for no-lock, legacy, and integration tests. Integration test verifies auto-enrollment with initTotalStaked(), retroactive rewards, and correct reward math across rotation.
Deploy veLikeRewardNoLock proxy with setVault, setLikecoin, and initTotalStaked calls included in the deployment script so that the reward contract is fully configured for reward rotation with auto-enrollment of existing stakers.
|
Updated the PR to better address the If the user has zero interaction with reward, i.e. not adding/claim/withdraw. Admin will need to sync before rotating out the reward contract. Otherwise, that user will not able to claim the rewards. So marketing-wise, we can ask user to have some kind of interaction with the veLike reward. or just do it silently on new reward is assigned. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| uint256 targetTime = block.timestamp; | ||
| if (targetTime > $.currentStakingCondition.endTime) { | ||
| targetTime = $.currentStakingCondition.endTime; | ||
| } | ||
| uint256 timePassed = targetTime - $.lastRewardTime; | ||
| uint256 newReward = timePassed * | ||
| _rewardPerTimeWithPrecision($.currentStakingCondition); |
There was a problem hiding this comment.
getPendingReward() can underflow and revert when queried before the reward period starts: timePassed = targetTime - lastRewardTime while lastRewardTime is set to startTime in addReward. Consider clamping targetTime to max(startTime, lastRewardTime) (similar to _updateVault) or early-returning calculatedReward when block.timestamp < startTime so reads are safe pre-start.
| if (startTime <= $.lastRewardTime) { | ||
| revert ErrConflictCondition(); | ||
| } | ||
| if (endTime < startTime) { | ||
| revert ErrConflictCondition(); | ||
| } | ||
| if (endTime < block.timestamp) { | ||
| revert ErrConflictCondition(); | ||
| } |
There was a problem hiding this comment.
In addReward, endTime == startTime is currently allowed (endTime < startTime), but _rewardPerTimeWithPrecision() later divides by (endTime - startTime) and will revert on zero-length periods. Reject endTime <= startTime (or handle zero-duration explicitly) to avoid a configuration that bricks the reward contract at runtime.
likecoin3/contracts/veLikeReward.sol
Outdated
| $.totalStaked -= amount; | ||
| $.stakerInfos[account].stakedAmount -= amount; |
There was a problem hiding this comment.
Do we need to check $.totalStaked >= amount;?
There was a problem hiding this comment.
It should keep track correctly, I updated the op manual. The amount should have sync via _syncStaker in new noLock reward. For the 1st vault, it should no longer call this after the interface update.
hmm, so you are right, this update is not necessary for the first lock, we can keep the old logic.
| _syncStaker(account); | ||
| _updateVault(); | ||
| _claimReward(account, false); | ||
| $.totalStaked -= amount; |
There was a problem hiding this comment.
not sure if we should check $.totalStaked >= amount
It will prevent operational error
Try to follow the official guide: https://hardhat.org/ignition/docs/guides/upgradeable-proxies Checkout the veLike and vLikeReward from 93da984 and move current code as V2
Decision notes: