QuantaPool v2: fixed-balance model, security fixes, Hyperion port#12
QuantaPool v2: fixed-balance model, security fixes, Hyperion port#12moscowchill merged 20 commits intomainfrom
Conversation
- balanceOf() now returns raw shares (stable, tax-friendly) - Added getQRLValue() for QRL equivalent display - Exchange rate calculated via totalPooledQRL/totalShares - Same security properties, cleaner tax implications Community feedback: rebasing creates potential taxable events
- Lido and Rocket Pool for liquid staking designs - The QRL Core Team for post-quantum blockchain infrastructure - Robyer for fixed-balance token model feedback
Added comprehensive test coverage for: stQRL-v2.sol (33 new tests): - Approve function and events - Transfer error paths (zero address, zero amount, insufficient balance) - TransferFrom error paths (insufficient allowance, unlimited allowance) - Pause affects transferFrom - Mint/burn error paths and events - Admin functions (transferOwnership, renounceOwnership) - getQRLValue direct tests DepositPool-v2.sol (38 new tests): - Deposit error paths (stQRL not set, zero amount) - Withdrawal error paths (zero shares, insufficient, already pending) - Cancel withdrawal errors - Validator funding errors and events - Admin functions (setStQRL, setMinDeposit, emergencyWithdraw) - Preview deposit - Receive function and fundWithdrawalReserve - Multi-user withdrawal queue - Proportional reward distribution - Slashing detection with events Fixed test_SlashingReducesWithdrawalAmount which was previously empty.
Addresses critical security vulnerabilities identified by audit: **H-01: First Depositor Share Inflation Attack (Donation Attack)** - Added VIRTUAL_SHARES and VIRTUAL_ASSETS constants (1e3) - Updated getSharesByPooledQRL() to use virtual offsets - Updated getPooledQRLByShares() to use virtual offsets - Updated getExchangeRate() for consistency - This prevents attackers from manipulating share pricing in an empty pool **H-02: Withdrawal Claim Accounting Discrepancy** - Refactored claimWithdrawal() to burn shares FIRST - Uses the actual burned QRL amount for all subsequent calculations - Ensures reserve check, accounting, and transfer use consistent values - Prevents potential insolvency from rate changes during claim Test suite updated: - Fixed mint/update order to match DepositPool behavior (mint first) - Changed exact equality to approximate equality for QRL value assertions - All 115 tests passing Security: Virtual shares make donation attacks economically unviable by creating a floor that ensures fair pricing even with near-empty pools.
M-03: Restrict emergencyWithdraw to only allow withdrawing excess balance - Added ExceedsRecoverableAmount error - Calculates recoverable amount: balance - totalPooledQRL - withdrawalReserve - Prevents owner from draining user funds - Added EmergencyWithdrawal event for transparency M-04: Full 32-byte withdrawal credentials verification in fundValidator - Verifies exact format: 0x01 + 11 zero bytes + contract address - Uses assembly for efficient calldata loading - Prevents validators from being funded with wrong withdrawal address Additional changes: - Multiple withdrawal requests per user (array-based storage) - Added getWithdrawalRequestCount() helper function - Added WithdrawalCancelled event - Made setStQRL() one-time only (StQRLAlreadySet error) - Removed unused WithdrawalPending error All 118 tests passing (55 stQRL + 63 DepositPool)
…lash M-1 audit finding: The status check happened AFTER changing status to Slashed, so the condition was always false and activeValidatorCount never decremented when slashing an active validator. Fix: Cache previousStatus before modifying, then check cached value. Also moves TestToken.sol to v1-deprecated (test helper, not protocol).
55 tests covering: - Initialization and state - Validator registration (success, auth, duplicates, invalid pubkey) - Activation (single and batch) - Exit request and completion - Slashing with M-1 fix verification (counter decrements correctly) - View functions (getValidator, getStats, getValidatorsByStatus) - Admin functions (setDepositPool, transferOwnership) - Full lifecycle tests - Fuzz tests for registration and slashing counter correctness
…tate N-1 audit finding: Counter was only decremented when slashing from Active state, but Exiting validators also count toward activeValidatorCount. This could leave the counter artificially high if validators were slashed while in the exit queue. Fix: Always decrement activeValidatorCount when slashing, since both Active and Exiting states are included in the count.
- Move outdated research docs to v1-deprecated: - rocketpool-reading-guide.md (initial research) - minipool-economics.md (unused minipool concept) - quantapool-research.md (references 40k QRL, outdated) - Add new docs/architecture.md reflecting current v2 implementation: - Fixed-balance token model (stQRL-v2) - Trustless reward sync (no oracle) - ValidatorManager lifecycle - 10,000 QRL validators - Security model and test coverage
Zond's mainnet_config.go specifies MaxEffectiveBalance: 40000 * 1e9, not 10,000 as previously used. This matches the original v1 contracts. Updated: - DepositPool-v2.sol: VALIDATOR_STAKE = 40_000 ether - ValidatorManager.sol: VALIDATOR_STAKE = 40_000 ether - Test files: deposit amounts for validator funding tests - docs/architecture.md: correct stake amount
QP-01: Add share locking to stQRL so users cannot transfer shares that are pending withdrawal. DepositPool locks on request, unlocks on claim/cancel. Adapted for array-based multi-request withdrawals — checks unlocked balance before allowing new withdrawal requests. QP-05: Add MIN_DEPOSIT_FLOOR (0.01 ether) so setMinDeposit cannot be set to zero. Defense-in-depth on top of the existing virtual shares inflation protection.
Replace constant MIN_DEPOSIT_FLOOR with adjustable minDepositFloor state variable so the owner can lower the deposit minimum post-deployment if QRL appreciates. Adds ABSOLUTE_MIN_DEPOSIT (0.001 QRL) as dust prevention.
…cture Move Solidity contracts to contracts/solidity/, add Hyperion (.hyp) ports under contracts/hyperion/ with pragma hyperion ^0.8.24. Update all test import paths for the new layout.
The README was out of date — it didn't cover the infrastructure (Terraform/Ansible), monitoring stack (Prometheus/Grafana/Alertmanager), key management tooling, audit-driven tests, CI, or project structure. Updated test count from 173 to 217, added completed roadmap items, and expanded the architecture diagram and security section. https://claude.ai/code/session_01JD9cmcCgf9dNcWVZ9CXbgj
- Fix VALIDATOR_STAKE (10k → 40k) and signature size (~2420 → 4595) in architecture.md - Update test counts to match actual suite (178 tests across 3 suites) - Replace ERC-20 with QRC-20 in docs and stQRL-v2.sol comments - Remove 7 audit PoC test files (39 tests) — bugs are fixed, core suite covers regressions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the QuantaPool protocol by transitioning to a fixed-balance liquid staking token, enhancing security against common attacks, and introducing a dedicated contract for robust validator lifecycle management. It also expands the project's reach by integrating a Hyperion port layer, ensuring broader compatibility and future-proofing. The changes aim to provide a more stable, secure, and maintainable foundation for QRL liquid staking. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
The pull request refactors the QuantaPool liquid staking protocol from a rebasing to a fixed-balance token model, introducing virtual shares to prevent first-depositor attacks and updating the withdrawal mechanism to support multiple requests per user. Key changes include modifying stQRL-v2.sol to return stable shares via balanceOf() and a fluctuating QRL equivalent via getQRLValue(), and enhancing DepositPool-v2.sol to manage a queue of withdrawal requests, lock shares during the request phase, and improve the emergencyWithdraw function. The ValidatorManager.sol contract was updated to correctly decrement the active validator count upon slashing. Documentation (README.md, docs/architecture.md) and test files were updated to reflect these changes, and new Hyperion-related files were added for cross-chain compatibility. A review comment highlights a discrepancy where the documentation incorrectly states shares are burned during withdrawal requests instead of locked, and another points out a potential denial-of-service vulnerability in the claimWithdrawal function due to an unbounded loop for skipping cancelled requests, which could lead to out-of-gas issues.
docs/architecture.md
Outdated
|
|
||
| **Withdrawal Flow:** | ||
| 1. User calls `requestWithdrawal(shares)` | ||
| 2. Shares burned, QRL amount calculated |
There was a problem hiding this comment.
The description of the withdrawal flow states that shares are burned during the request. However, in the implementation, shares are only locked at this stage and are burned later during the claim. This could be misleading for developers and auditors. I recommend updating this step to clarify that shares are locked, not burned, upon request.
| while (requestIndex < totalRequests && withdrawalRequests[msg.sender][requestIndex].shares == 0) { | ||
| requestIndex++; | ||
| } |
There was a problem hiding this comment.
The while loop to skip cancelled withdrawal requests could lead to a denial-of-service condition if a user has a large number of consecutive cancelled requests. The transaction could run out of gas, preventing the user from claiming any subsequent valid withdrawals. Consider adding a limit to the number of iterations or allowing the user to specify a starting index to claim from to mitigate this potential issue.
- architecture.md: shares are locked at request time, burned at claim (not burned at request) - DepositPool-v2.sol: add comment clarifying the cancelled-request skip loop is self-bounded Addresses Gemini Code Assist review comments on PR #12. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
balanceOf()) with a separategetQRLValue()for QRL equivalent, matching wstETH design (tax-friendly)sync-hyperion.jsgenerates.hypmirrors from Solidity sources, with dedicated compile and deploy scripts for Zondcontracts/solidity/, v1 contracts tov1-deprecated/, docs reorganizedKnown blockers (not in scope)
Beacon deposit contract reverts on current testnet — documented in
infrastructure/docs/validator-integration.md. Root cause is testnet infra, not QuantaPool contracts. New testnet expected soon.Test plan
forge build— compiles with no errorsforge test— 178/178 passing, 0 failed🤖 Generated with Claude Code