Add Delegation Pool contract #449
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive Delegation Pool contract that allows users to pool their SQT tokens for delegation to indexers, with automated reward management and fee collection. The pool manager can allocate funds to indexers to optimize rewards while users receive ERC20 shares representing their stake.
Key changes:
- Adds a new DelegationPool contract with delegation, undelegation, and withdrawal functionality
- Implements era-based share pricing to prevent reward manipulation attacks
- Provides comprehensive test coverage with 1,340 test lines and mock contracts
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| contracts/DelegationPool.sol | Main delegation pool contract implementing ERC20 shares, manager functions, and auto-compounding |
| test/DelegationPool.test.ts | Comprehensive test suite covering all functionality with edge cases and integration tests |
| contracts/mocks/MockStakingManager.sol | Mock contract for testing auto-compound functionality with simulated rewards |
| contracts/interfaces/IStakingManager.sol | Extended interface with delegation and reward management functions |
| publish/revertcode.json | Added error codes (DP001-DP015) for delegation pool specific errors |
| contracts/RewardsPool.sol | Minor type consistency fixes (uint to uint256) |
| contracts/RewardsBooster.sol | Minor type consistency fix (uint to uint256) |
| CLAUDE.md | Added development documentation for AI assistance |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| /** | ||
| * @dev Automatic compound rewards for all active delegations | ||
| */ | ||
| function autoCompound() external { | ||
| require(activeIndexers.length > 0, 'DP013'); | ||
|
|
||
| // Update share price for new era BEFORE claiming rewards | ||
| // This ensures any new deposits use the old price (no reward benefit) | ||
| _updateSharePriceIfNeeded(); | ||
|
|
||
| IStakingManager stakingManager = IStakingManager( | ||
| settings.getContractAddress(SQContracts.StakingManager) | ||
| ); | ||
| IERC20 sqToken = IERC20(settings.getContractAddress(SQContracts.SQToken)); | ||
|
|
||
| uint256 rewardsBefore = sqToken.balanceOf(address(this)); | ||
| stakingManager.batchStakeReward(activeIndexers); | ||
| uint256 rewardsAfter = sqToken.balanceOf(address(this)); | ||
|
|
||
| uint256 totalRewards = rewardsAfter - rewardsBefore; | ||
|
|
||
| if (totalRewards == 0) { | ||
| return; // No rewards to compound | ||
| } | ||
|
|
||
| // Calculate and deduct fees from rewards | ||
| uint256 feeAmount = 0; | ||
| if (feePerMill > 0) { | ||
| feeAmount = (totalRewards * feePerMill) / PER_MILL; | ||
| accumulatedFees += feeAmount; | ||
| emit FeesDeducted(totalRewards, feeAmount); | ||
| } | ||
|
|
||
| // Add remaining rewards to available assets for compounding | ||
| uint256 compoundAmount = totalRewards - feeAmount; | ||
| availableAssets += compoundAmount; |
There was a problem hiding this comment.
In autoCompound the contract locks the era share price via _updateSharePriceIfNeeded() before calling batchStakeReward and never updates it again once availableAssets increases (lines 488‑520). Deposits executed later in the same era continue minting shares with the stale currentEraSharePrice, so newcomers can buy shares at the pre‑reward rate and capture a portion of the freshly compounded rewards they didn’t contribute. The share price should be recalculated after rewards are added (or rewards should mint new shares) so asset-per-share stays accurate within the era.
Useful? React with 👍 / 👎.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughIntroduces a new DelegationPool contract enabling pooled SQT delegation with ERC20-compatible pool shares, complete with delegation lifecycle management, fee collection, automatic compounding, and manager operations. Includes supporting interfaces, comprehensive testing, deployment automation, error codes, and documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DelegationPool as DelegationPool<br/>(Pool)
participant StakingManager as StakingManager
participant RewardsDistributor
rect rgb(200, 220, 255)
Note over User,RewardsDistributor: Delegation Flow
User->>DelegationPool: delegate(_amount)
DelegationPool->>DelegationPool: Calculate shares<br/>(using era-locked price)
DelegationPool->>DelegationPool: Mint pool shares
DelegationPool->>DelegationPool: Update availableAssets
end
rect rgb(220, 200, 255)
Note over User,StakingManager: Manager Delegation (Admin)
DelegationPool->>StakingManager: delegate(_runner, _amount)
StakingManager->>StakingManager: Record delegation
end
rect rgb(200, 255, 220)
Note over User,RewardsDistributor: Auto-Compound
DelegationPool->>RewardsDistributor: getPendingRewards() for all runners
RewardsDistributor-->>DelegationPool: Return rewards
DelegationPool->>DelegationPool: Deduct fees
DelegationPool->>DelegationPool: Add net rewards to availableAssets
end
rect rgb(255, 220, 200)
Note over User,StakingManager: Undelegation Flow
User->>DelegationPool: undelegate(_shareAmount)
DelegationPool->>DelegationPool: Burn shares<br/>Calculate SQT amount
alt Assets Available
DelegationPool->>DelegationPool: Direct withdrawal
else Assets Insufficient
DelegationPool->>StakingManager: undelegate(_runner, _amount)
DelegationPool->>DelegationPool: Schedule pending undelegation<br/>Apply unbond fee
end
end
rect rgb(240, 220, 255)
Note over User,StakingManager: Withdrawal (Post-Lock)
User->>DelegationPool: Claim mature withdrawals
DelegationPool->>User: Transfer SQT
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
contracts/DelegationPool.sol (1)
508-546: Share price stays stale after compoundingLine 513 locks the era price before rewards are swept, and nothing refreshes
currentEraSharePriceafteravailableAssetsgrows. Any deposit later in the same era mints shares at the pre-reward rate and skims part of the compounded rewards from existing holders. You either need to recompute the share price once rewards are credited (without bumpinglastPriceUpdateEra) or mint representative shares for the rewards so that asset-per-share remains consistent inside the era.// Add remaining rewards to available assets for compounding uint256 compoundAmount = totalRewards - feeAmount; availableAssets += compoundAmount; + + // Keep the era price in sync with the enlarged asset base + if (totalSupply() > 0) { + uint256 totalAssets = _calculateTotalAssets(); + currentEraSharePrice = (totalAssets * 1e18) / totalSupply(); + totalAssetsAtLastUpdate = totalAssets; + totalSharesAtLastUpdate = totalSupply(); + }
🧹 Nitpick comments (2)
CLAUDE.md (2)
58-66: Specify language for fenced code block.The fenced code block showing the directory structure should have a language identifier (e.g.,
textorplaintext) for proper rendering.Apply this diff:
-``` +```text /contracts/ ├── [Root contracts] - 35+ core contracts (Settings, Staking, IndexerRegistry, etc.)
85-85: Minor grammar improvement.Consider using a hyphen to join "ethers-rs based" as "ethers-rs-based" for grammatical correctness.
Apply this diff:
-- **Rust SDK**: `/rust/` - ethers-rs based SDK for Rust applications +- **Rust SDK**: `/rust/` - ethers-rs-based SDK for Rust applications
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
CLAUDE.md(1 hunks)contracts/DelegationPool.sol(1 hunks)contracts/RewardsBooster.sol(1 hunks)contracts/RewardsPool.sol(3 hunks)contracts/interfaces/IStakingManager.sol(1 hunks)contracts/mocks/MockStakingManager.sol(1 hunks)package.json(1 hunks)publish/revertcode.json(1 hunks)scripts/contracts.ts(2 hunks)scripts/delegationPool.ts(1 hunks)scripts/deployContracts.ts(10 hunks)src/contracts.ts(2 hunks)src/sdk.ts(2 hunks)src/types.ts(3 hunks)test/DelegationPool.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
scripts/delegationPool.ts (3)
scripts/logger.ts (1)
getLogger(20-22)src/types.ts (2)
ContractDeployment(86-89)CONTRACT_FACTORY(107-146)scripts/deployContracts.ts (1)
deployProxy(158-184)
test/DelegationPool.test.ts (2)
test/helper.ts (6)
Wallet(40-40)registerRunner(148-163)etherParse(119-122)timeTravel(47-51)constants(36-36)startNewEra(314-319)scripts/deployContracts.ts (1)
deployContracts(343-635)
scripts/deployContracts.ts (1)
test/helper.ts (1)
Wallet(40-40)
🪛 LanguageTool
CLAUDE.md
[grammar] ~85-~85: Use a hyphen to join words.
Context: ...SDK - Rust SDK: /rust/ - ethers-rs based SDK for Rust applications - **Cont...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
CLAUDE.md
58-58: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (7)
contracts/RewardsPool.sol (1)
282-285: Consistent uint256 indexingGood call aligning the stored counters and loop index to
uint256; it keeps the array walk in_batchCollectconsistent with the struct storage and avoids implicit casts. Looks solid.contracts/RewardsBooster.sol (1)
432-432: LGTM: Improved type consistency.The change from
uinttouint256makes the type explicit and aligns with Solidity best practices for clarity.package.json (1)
24-24: LGTM: Deployment script properly configured.The new deployment script follows the existing pattern and conventions used by other deployment scripts in the project.
publish/revertcode.json (1)
225-239: LGTM: Error codes well-defined.The new DelegationPool error codes (DP001-DP015) are clearly defined with descriptive messages that will help with debugging and user feedback.
src/types.ts (1)
48-48: LGTM: DelegationPool properly integrated into type system.The additions correctly expose DelegationPool through the factory mapping and enum, following the established patterns for other contracts.
Also applies to: 145-145, 170-170
src/sdk.ts (1)
36-36: LGTM: SDK property correctly added.The delegationPool property is properly integrated into the ContractSDK class, following the same pattern as other contract properties. The dynamic initialization in
_init()will handle the setup automatically.Also applies to: 79-79
contracts/interfaces/IStakingManager.sol (1)
11-23: All implementations of IStakingManager already provide the required functions.Verification confirms both implementations are complete:
- MockStakingManager.sol: all 7 functions present (delegate, undelegate, redelegate, cancelUnbonding, widthdraw, stakeReward, batchStakeReward)
- StakingManager.sol: all 7 functions present
No compilation errors will occur from missing implementations.
| uint256 feeFromAvailable = MathUtil.mulDiv( | ||
| unbondFeeRate, | ||
| amountFromAvailable, | ||
| PER_MILL | ||
| ); | ||
| uint256 feeFromIndexers = MathUtil.mulDiv(unbondFeeRate, amountFromIndexers, PER_MILL); | ||
|
|
||
| // Calculate net amounts after fees | ||
| uint256 netFromAvailable = amountFromAvailable - feeFromAvailable; | ||
| uint256 netFromIndexers = amountFromIndexers - feeFromIndexers; | ||
|
|
||
| // Track fees: accumulated (from available) and expected (from indexers) | ||
| accumulatedUnbondFees += feeFromAvailable; | ||
| expectedUnbondFees += feeFromIndexers; | ||
| emit UnbondFeeTracked(feeFromIndexers, expectedUnbondFees); | ||
|
|
There was a problem hiding this comment.
expectedUnbondFees only ever grows
Lines 313-315 add indexer fees to expectedUnbondFees, but nothing ever subtracts from this accumulator—not when deposits backfill the pending undelegation, not when the manager actually withdraws, and not when users are paid. As a result _calculateTotalAssets() (Line 753) permanently understates assets, depressing the share price and letting new deposits mint extra shares. Please wire in the inverse operation (e.g., track per-request fees and clear them when the corresponding undelegation is cancelled or completed) so the accounting stays balanced.
// Track fees: accumulated (from available) and expected (from indexers)
accumulatedUnbondFees += feeFromAvailable;
expectedUnbondFees += feeFromIndexers;
emit UnbondFeeTracked(feeFromIndexers, expectedUnbondFees);
+ unbondingRequests[msg.sender][unbondingRequests[msg.sender].length - 1]
+ .expectedFeeFromIndexers = feeFromIndexers;- if (pendingUndelegationsForUsers > 0) {
+ if (pendingUndelegationsForUsers > 0) {
// Use new deposit to cover pending undelegations first
if (_amount >= pendingUndelegationsForUsers) {
// Fully cover pending undelegations
availableAssets += (_amount - pendingUndelegationsForUsers);
+ expectedUnbondFees -= _claimableFeeOffset(_amount - (_amount - pendingUndelegationsForUsers));
pendingUndelegationsForUsers = 0;- stakingManager.undelegate(_runner, _amount);
+ stakingManager.undelegate(_runner, _amount);
+ expectedUnbondFees -= _expectedFeeFor(_runner, _amount);Committable suggestion skipped: line range outside the PR's diff.
|
|
||
| function cancelUnbonding(uint256 unbondReqId) external; | ||
|
|
||
| function widthdraw() external; |
There was a problem hiding this comment.
Fix the function name typo.
The function is misspelled as widthdraw() and should be withdraw().
Apply this diff:
- function widthdraw() external;
+ function withdraw() external;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function widthdraw() external; | |
| function withdraw() external; |
🤖 Prompt for AI Agents
In contracts/interfaces/IStakingManager.sol around line 19, the function name is
misspelled as "widthdraw()" — change the declaration to the correct spelling
"withdraw()" so the interface matches expected naming; update any implementing
contracts or references if necessary to use withdraw() instead of widthdraw().
| Airdropper__factory, | ||
| SubnetProjectVote, | ||
| SubnetProjectVote__factory, | ||
| DelegationPool, |
There was a problem hiding this comment.
🧩 Analysis chain
Verify if DelegationPool should be upgradeable.
DelegationPool was added to the Contracts type but not to the UPGRADEBAL_CONTRACTS mapping. If DelegationPool uses the upgradeable proxy pattern, it should be added to UPGRADEBAL_CONTRACTS.
Run the following script to check if DelegationPool is upgradeable:
Also applies to: 110-110
🏁 Script executed:
#!/bin/bash
# Check if DelegationPool uses upgradeable pattern
echo "=== Checking DelegationPool contract ==="
rg -n "Initializable|Upgradeable|__.*_init" contracts/DelegationPool.sol -A2 -B2
echo ""
echo "=== Checking for proxy deployment in scripts ==="
rg -n "deployProxy|upgradeProxy" scripts/delegationPool.ts -C3Length of output: 1627
I can confirm DelegationPool is an upgradeable contract. Now I need to verify if it's properly listed in the UPGRADEBAL_CONTRACTS mapping in scripts/contracts.ts.
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Check the Contracts type and UPGRADEBAL_CONTRACTS mapping
echo "=== Checking Contracts type (around line 70) ==="
sed -n '60,80p' scripts/contracts.ts
echo ""
echo "=== Checking UPGRADEBAL_CONTRACTS mapping (around line 110) ==="
sed -n '100,150p' scripts/contracts.ts
echo ""
echo "=== Searching for UPGRADEBAL_CONTRACTS definition ==="
rg -n "UPGRADEBAL_CONTRACTS|UPGRADEABLE_CONTRACTS" scripts/contracts.ts -A20Length of output: 4831
I need to verify if DelegationPool is actually in the UPGRADEBAL_CONTRACTS mapping:
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Check if DelegationPool is in UPGRADEBAL_CONTRACTS mapping
echo "=== Searching for DelegationPool in UPGRADEBAL_CONTRACTS mapping ==="
rg -n "DelegationPool" scripts/contracts.ts
echo ""
echo "=== Showing full UPGRADEBAL_CONTRACTS mapping ==="
awk '/export const UPGRADEBAL_CONTRACTS/,/^};/' scripts/contracts.ts | tail -20Length of output: 1524
Add DelegationPool to UPGRADEBAL_CONTRACTS mapping.
DelegationPool is an upgradeable contract (uses Initializable, OwnableUpgradeable, ReentrancyGuardUpgradeable, and ERC20Upgradeable with initialize() function), and is deployed via deployProxy() in scripts/delegationPool.ts. However, it is missing from the UPGRADEBAL_CONTRACTS mapping in scripts/contracts.ts. Add the entry:
DelegationPool: [CONTRACTS.DelegationPool, DelegationPool__factory],
🤖 Prompt for AI Agents
In scripts/contracts.ts around line 70, the UPGRADEBAL_CONTRACTS mapping is
missing the DelegationPool entry; add an entry mapping the key "DelegationPool"
to the deployed contract and its factory so upgradeable deployments are
recognized. Update the mapping to include DelegationPool:
[CONTRACTS.DelegationPool, DelegationPool__factory] (ensure DelegationPool and
DelegationPool__factory are imported at the top if not already).
| await deployDelegationPool({ | ||
| wallet: connectedWallet, | ||
| deployment, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Persist the DelegationPool deployment metadata
Lines 41-45 reload the publish JSON, deploy the proxy, and exit without ever updating the deployment file. Downstream scripts continue to see DelegationPool as “not deployed”, so the next run will try to redeploy (or fail when looking up deployment.child.DelegationPool). Please store the new address, logic (innerAddress), and metadata in deployment.child.DelegationPool and call saveDeployment before returning. A sketch:
-import { deployProxy } from './deployContracts';
+import { deployProxy, saveDeployment } from './deployContracts';
+import CONTRACTS from '../src/contracts';
+import sha256 from 'sha256';
@@
-export async function deployDelegationPool({ wallet, deployment }: { wallet: Wallet; deployment: ContractDeployment }) {
+export async function deployDelegationPool({ wallet, deployment }: { wallet: Wallet; deployment: ContractDeployment }) {
@@
- const [contract, innerAddress] = await deployProxy(proxyAdmin, CONTRACT_FACTORY.DelegationPool, wallet, confirms);
+ const [contract, innerAddress] = await deployProxy(proxyAdmin, CONTRACT_FACTORY.DelegationPool, wallet, confirms);
@@
- return contract;
+ return { contract, innerAddress };
}
@@
- await deployDelegationPool({
+ const { contract, innerAddress } = await deployDelegationPool({
wallet: connectedWallet,
deployment,
});
+
+ deployment.child.DelegationPool = {
+ address: contract.address,
+ innerAddress,
+ bytecodeHash: sha256(Buffer.from(CONTRACTS.DelegationPool.bytecode.slice(2), 'hex')),
+ lastUpdate: new Date().toUTCString(),
+ };
+ saveDeployment(name, deployment);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/delegationPool.ts around lines 41 to 45, the deployment function
calls await deployDelegationPool(...) and returns without persisting the new
proxy address, logic (innerAddress), and any returned metadata; update
deployment.child.DelegationPool with the returned values (address,
innerAddress/logic, and metadata) from deployDelegationPool, then call
saveDeployment(deployment) to write the updated deployment file before returning
so downstream scripts see the contract as deployed.
| describe.only('managerUndelegate()', () => { | ||
| beforeEach(async () => { | ||
| // Setup: Delegate to runner first | ||
| await delegationPool.connect(poolManager).managerDelegate(runner1.address, etherParse('2000')); | ||
| }); | ||
|
|
||
| it('should allow manager to undelegate from runner', async () => { | ||
| const undelegateAmount = etherParse('1000'); | ||
|
|
||
| // Make sure all available assets are delegated | ||
| await delegationPool | ||
| .connect(poolManager) | ||
| .managerDelegate(runner2.address, await delegationPool.availableAssets()); | ||
| // User undelegates to make a pending undelegation | ||
| await delegationPool.connect(user1).undelegate(undelegateAmount); | ||
|
|
||
| await expect(delegationPool.connect(poolManager).managerUndelegate(runner1.address, undelegateAmount)) | ||
| .to.emit(delegationPool, 'ManagerUndelegated') | ||
| .withArgs(runner1.address, undelegateAmount); | ||
|
|
||
| expect(await delegationPool.getDelegatedToIndexer(runner1.address)).to.equal(etherParse('1000')); | ||
| }); | ||
|
|
||
| it('should remove indexer from active list when delegation becomes zero', async () => { | ||
| const amount = etherParse('2000'); | ||
| await delegationPool | ||
| .connect(poolManager) | ||
| .managerDelegate(runner2.address, await delegationPool.availableAssets()); | ||
| await delegationPool.connect(user1).undelegate(amount); | ||
| await delegationPool.connect(poolManager).managerUndelegate(runner1.address, amount); | ||
|
|
||
| expect(await delegationPool.getDelegatedToIndexer(runner1.address)).to.equal(0); | ||
| expect(await delegationPool.isActiveIndexer(runner1.address)).to.be.false; | ||
| expect(await delegationPool.getActiveIndexersCount()).to.equal(1); | ||
| }); | ||
|
|
||
| it('should reject undelegation exceeding delegated amount', async () => { | ||
| const excessiveAmount = etherParse('3000'); | ||
| await expect( | ||
| delegationPool.connect(poolManager).managerUndelegate(runner1.address, excessiveAmount) | ||
| ).to.be.revertedWith('DP010'); | ||
| }); | ||
|
|
||
| it('should reduce the pendingUndelegationsForUsers after manager undelegating', async () => { | ||
| const amount = etherParse('3000'); | ||
| await delegationPool.connect(poolManager).managerDelegate(runner1.address, amount); | ||
|
|
||
| // All funds should be delegated | ||
| expect(await delegationPool.availableAssets()).to.equal(0); | ||
| expect(await delegationPool.pendingUndelegationsForUsers()).to.equal(0); | ||
|
|
||
| await delegationPool.connect(user1).undelegate(amount); | ||
|
|
||
| expect(await delegationPool.pendingUndelegationsForUsers()).to.equal(amount); | ||
|
|
||
| await delegationPool.connect(poolManager).managerUndelegate(runner1.address, amount); | ||
| expect(await delegationPool.pendingUndelegationsForUsers()).to.equal(0); | ||
| }); | ||
|
|
||
| // If manager undelegates before user undelegation, pendingUndelegationsForUsers should not increase | ||
| it('not allow manager to undelegate more than users have requested', async () => { | ||
| const amount = etherParse('3000'); | ||
| // Delegate all funds | ||
| await delegationPool.connect(poolManager).managerDelegate(runner1.address, amount); | ||
|
|
||
| // All funds should be delegated | ||
| expect(await delegationPool.availableAssets()).to.equal(0); | ||
| expect(await delegationPool.pendingUndelegationsForUsers()).to.equal(0); | ||
|
|
||
| await expect( | ||
| delegationPool.connect(poolManager).managerUndelegate(runner1.address, amount) | ||
| ).to.be.revertedWith('DP015'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Remove .only to restore the full test suite
Line 509 wraps managerUndelegate() in describe.only, so Mocha will skip every other DelegationPool test. This hides regressions and will make CI report green while most coverage never runs. Please drop the .only.
- describe.only('managerUndelegate()', () => {
+ describe('managerUndelegate()', () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe.only('managerUndelegate()', () => { | |
| beforeEach(async () => { | |
| // Setup: Delegate to runner first | |
| await delegationPool.connect(poolManager).managerDelegate(runner1.address, etherParse('2000')); | |
| }); | |
| it('should allow manager to undelegate from runner', async () => { | |
| const undelegateAmount = etherParse('1000'); | |
| // Make sure all available assets are delegated | |
| await delegationPool | |
| .connect(poolManager) | |
| .managerDelegate(runner2.address, await delegationPool.availableAssets()); | |
| // User undelegates to make a pending undelegation | |
| await delegationPool.connect(user1).undelegate(undelegateAmount); | |
| await expect(delegationPool.connect(poolManager).managerUndelegate(runner1.address, undelegateAmount)) | |
| .to.emit(delegationPool, 'ManagerUndelegated') | |
| .withArgs(runner1.address, undelegateAmount); | |
| expect(await delegationPool.getDelegatedToIndexer(runner1.address)).to.equal(etherParse('1000')); | |
| }); | |
| it('should remove indexer from active list when delegation becomes zero', async () => { | |
| const amount = etherParse('2000'); | |
| await delegationPool | |
| .connect(poolManager) | |
| .managerDelegate(runner2.address, await delegationPool.availableAssets()); | |
| await delegationPool.connect(user1).undelegate(amount); | |
| await delegationPool.connect(poolManager).managerUndelegate(runner1.address, amount); | |
| expect(await delegationPool.getDelegatedToIndexer(runner1.address)).to.equal(0); | |
| expect(await delegationPool.isActiveIndexer(runner1.address)).to.be.false; | |
| expect(await delegationPool.getActiveIndexersCount()).to.equal(1); | |
| }); | |
| it('should reject undelegation exceeding delegated amount', async () => { | |
| const excessiveAmount = etherParse('3000'); | |
| await expect( | |
| delegationPool.connect(poolManager).managerUndelegate(runner1.address, excessiveAmount) | |
| ).to.be.revertedWith('DP010'); | |
| }); | |
| it('should reduce the pendingUndelegationsForUsers after manager undelegating', async () => { | |
| const amount = etherParse('3000'); | |
| await delegationPool.connect(poolManager).managerDelegate(runner1.address, amount); | |
| // All funds should be delegated | |
| expect(await delegationPool.availableAssets()).to.equal(0); | |
| expect(await delegationPool.pendingUndelegationsForUsers()).to.equal(0); | |
| await delegationPool.connect(user1).undelegate(amount); | |
| expect(await delegationPool.pendingUndelegationsForUsers()).to.equal(amount); | |
| await delegationPool.connect(poolManager).managerUndelegate(runner1.address, amount); | |
| expect(await delegationPool.pendingUndelegationsForUsers()).to.equal(0); | |
| }); | |
| // If manager undelegates before user undelegation, pendingUndelegationsForUsers should not increase | |
| it('not allow manager to undelegate more than users have requested', async () => { | |
| const amount = etherParse('3000'); | |
| // Delegate all funds | |
| await delegationPool.connect(poolManager).managerDelegate(runner1.address, amount); | |
| // All funds should be delegated | |
| expect(await delegationPool.availableAssets()).to.equal(0); | |
| expect(await delegationPool.pendingUndelegationsForUsers()).to.equal(0); | |
| await expect( | |
| delegationPool.connect(poolManager).managerUndelegate(runner1.address, amount) | |
| ).to.be.revertedWith('DP015'); | |
| }); | |
| }); | |
| describe('managerUndelegate()', () => { | |
| beforeEach(async () => { | |
| // Setup: Delegate to runner first | |
| await delegationPool.connect(poolManager).managerDelegate(runner1.address, etherParse('2000')); | |
| }); | |
| it('should allow manager to undelegate from runner', async () => { | |
| const undelegateAmount = etherParse('1000'); | |
| // Make sure all available assets are delegated | |
| await delegationPool | |
| .connect(poolManager) | |
| .managerDelegate(runner2.address, await delegationPool.availableAssets()); | |
| // User undelegates to make a pending undelegation | |
| await delegationPool.connect(user1).undelegate(undelegateAmount); | |
| await expect(delegationPool.connect(poolManager).managerUndelegate(runner1.address, undelegateAmount)) | |
| .to.emit(delegationPool, 'ManagerUndelegated') | |
| .withArgs(runner1.address, undelegateAmount); | |
| expect(await delegationPool.getDelegatedToIndexer(runner1.address)).to.equal(etherParse('1000')); | |
| }); | |
| it('should remove indexer from active list when delegation becomes zero', async () => { | |
| const amount = etherParse('2000'); | |
| await delegationPool | |
| .connect(poolManager) | |
| .managerDelegate(runner2.address, await delegationPool.availableAssets()); | |
| await delegationPool.connect(user1).undelegate(amount); | |
| await delegationPool.connect(poolManager).managerUndelegate(runner1.address, amount); | |
| expect(await delegationPool.getDelegatedToIndexer(runner1.address)).to.equal(0); | |
| expect(await delegationPool.isActiveIndexer(runner1.address)).to.be.false; | |
| expect(await delegationPool.getActiveIndexersCount()).to.equal(1); | |
| }); | |
| it('should reject undelegation exceeding delegated amount', async () => { | |
| const excessiveAmount = etherParse('3000'); | |
| await expect( | |
| delegationPool.connect(poolManager).managerUndelegate(runner1.address, excessiveAmount) | |
| ).to.be.revertedWith('DP010'); | |
| }); | |
| it('should reduce the pendingUndelegationsForUsers after manager undelegating', async () => { | |
| const amount = etherParse('3000'); | |
| await delegationPool.connect(poolManager).managerDelegate(runner1.address, amount); | |
| // All funds should be delegated | |
| expect(await delegationPool.availableAssets()).to.equal(0); | |
| expect(await delegationPool.pendingUndelegationsForUsers()).to.equal(0); | |
| await delegationPool.connect(user1).undelegate(amount); | |
| expect(await delegationPool.pendingUndelegationsForUsers()).to.equal(amount); | |
| await delegationPool.connect(poolManager).managerUndelegate(runner1.address, amount); | |
| expect(await delegationPool.pendingUndelegationsForUsers()).to.equal(0); | |
| }); | |
| // If manager undelegates before user undelegation, pendingUndelegationsForUsers should not increase | |
| it('not allow manager to undelegate more than users have requested', async () => { | |
| const amount = etherParse('3000'); | |
| // Delegate all funds | |
| await delegationPool.connect(poolManager).managerDelegate(runner1.address, amount); | |
| // All funds should be delegated | |
| expect(await delegationPool.availableAssets()).to.equal(0); | |
| expect(await delegationPool.pendingUndelegationsForUsers()).to.equal(0); | |
| await expect( | |
| delegationPool.connect(poolManager).managerUndelegate(runner1.address, amount) | |
| ).to.be.revertedWith('DP015'); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In test/DelegationPool.test.ts around lines 509 to 582, the top-level describe
for managerUndelegate() uses describe.only which limits Mocha to run only this
block; remove the .only so the test suite runs all tests. Edit the line to use
describe(...) (without .only), save, and re-run the test suite/CI to ensure full
coverage.
Implements a Delegation Pool contract
Overview
A Delegation Pool contract that allows aggregation of SQT to be pooled together and used to delegate to indexers.
Delegation should behave the same as if the user had delegated directly to an indexer. There is a manager (owner) of the pool
that determines the delegation strategy to indexers.
Details
Delegation
Users delegate their SQT to the pool and receive pool shares (ERC20 tokens) representing their stake in the pool. The pool manager can then
allocate those funds to the indexers they choose to optimise rewards.
Undelegation
Users can then trigger undelegation which will burn their shares and
start a withdrawal process that then allows the user to withdraw their SQT after the lock period which is the same duration as if they had
directly delegated.
Withdrawls
After the lock period, users can withdraw their SQT and rewards from the pool, minus and pool and unbonding fees.
Managers
The pool manager is responsible for making sure the available assets in the pool are delegated to indexers to earn rewards.
The manager can do all the same functionality as delegating directly to an indexer such as delegate, undelegate, redelegate and claim rewards.
The manager can also trigger auto-compounding of rewards which claims rewards from all indexers as well as set a reward fee.
Summary by CodeRabbit
New Features
Documentation
Tests