docs #61
docs #61
Conversation
WalkthroughThe changes introduce significant refactoring, documentation, and feature additions to the Changes
Sequence Diagram(s)sequenceDiagram
participant Manager
participant HyperfundFactory
participant Hyperfund
participant Hyperstaker
Manager->>HyperfundFactory: createProject(hypercertTypeId, admin, manager, pauser, upgrader)
HyperfundFactory->>Hyperfund: deploy(hypercertTypeId, admin, manager, pauser, upgrader)
HyperfundFactory->>Hyperstaker: deploy(hypercertTypeId, admin, manager, pauser, upgrader)
HyperfundFactory-->>Manager: return (Hyperfund, Hyperstaker addresses)
sequenceDiagram
participant Manager
participant Hyperfund
Manager->>Hyperfund: nonfinancialContribution(contributor, units)
Hyperfund->>Hyperfund: _nonfinancialContribution(contributor, units)
Hyperfund->>Hyperfund: _mintFraction(contributor, units)
Hyperfund-->>Manager: Hypercert fraction issued
sequenceDiagram
participant User
participant Hyperstaker
User->>Hyperstaker: stake(hypercertId)
Hyperstaker->>Hyperstaker: record stake
User->>Hyperstaker: claimReward(hypercertId, roundId)
Hyperstaker->>Hyperstaker: check eligibility and claim status
Hyperstaker->>User: transfer reward
Possibly related PRs
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (6)
src/HyperfundFactory.sol (2)
62-79:⚠️ Potential issueValidate all role addresses, not just
managerOnly
manageris checked againstaddress(0). Supplying a zero address foradmin,pauser, orupgraderwould brick their respective functionality and make the proxy undeployable if an upgrade or pause is ever required.- require(manager != address(0), InvalidAddress()); + require( + admin != address(0) && + manager != address(0) && + pauser != address(0) && + upgrader != address(0), + InvalidAddress() + );
125-150: 🛠️ Refactor suggestion
createProjectmisses two critical checks & one approval call
hyperstakers[hypercertTypeId]is not verified before deployment – you can overwrite an existing Hyperstaker.- The Hyperfund created inside this helper never receives
setApprovalForAll, leaving it unable to split/burn Hypercerts.- Same zero-address discussion as in
createHyperfund.- require(hyperfunds[hypercertTypeId] == false, AlreadyDeployed()); + require( + hyperfunds[hypercertTypeId] == false && + hyperstakers[hypercertTypeId] == false, + AlreadyDeployed() + ); ... - hyperfund = address(hyperfundProxy); + IHypercertToken(hypercertMinter).setApprovalForAll( + address(hyperfundProxy), + true + ); + hyperfund = address(hyperfundProxy);src/Hyperstaker.sol (2)
102-118:⚠️ Potential issueGuard against zero reward & duplicate native/ERC20 mismatch
setRewardallows_rewardAmount == 0, creating a round that every staker can claim for zero value but still paying gas.- Native branch: a malicious caller can send native value and specify an ERC20 token to lock user ether inside the contract forever (or vice-versa).
function setReward(address _rewardToken, uint256 _rewardAmount) external payable onlyRole(MANAGER_ROLE) { + require(_rewardAmount > 0, IncorrectRewardAmount(0, 1)); + require( + (_rewardToken == address(0) && msg.value == _rewardAmount) || + (_rewardToken != address(0) && msg.value == 0), + IncorrectRewardAmount(msg.value, _rewardAmount) + );
160-178: 🛠️ Refactor suggestion
claimReward: missing round-index & bitmap bounds checksAccessing
rounds[_roundId]reverts for out-of-range indices, but the error will be a generic VM revert.
Additionally, bit-shifting1 << _roundIdsilently returns0for_roundId >= 256, breaking the claimed-rounds logic.function claimReward(uint256 _hypercertId, uint256 _roundId) external whenNotPaused { + require(_roundId < rounds.length, RoundNotSet()); + require(_roundId < 256, "Round id exceeds bitmap size");src/Hyperfund.sol (2)
154-172:⚠️ Potential issuePrevent zero-unit mints & require allow-listed token in
fundIf
multiplier < 0and_amount < abs(multiplier)the computedunitsbecomes0, yet the user’s tokens are accepted and an empty split occurs.
Alsoredeemexpects the same token to be allow-listed;fundshould enforce it.- uint256 units = _tokenAmountToUnits(_token, _amount); + require(tokenMultipliers[_token] != 0, TokenNotAllowlisted()); + uint256 units = _tokenAmountToUnits(_token, _amount); + require(units != 0, InvalidAmount());
179-195: 🛠️ Refactor suggestion
redeemdoes not validate token allow-listingA user can pass an un-listed token, receive
tokenAmount == 0, burn their fraction and lose units.- uint256 tokenAmount = _unitsToTokenAmount(_token, units); + require(tokenMultipliers[_token] != 0, TokenNotAllowlisted()); + uint256 tokenAmount = _unitsToTokenAmount(_token, units);
🧹 Nitpick comments (2)
src/HyperfundFactory.sol (1)
15-18: Store addresses, not booleans, for easier discovery
mapping(uint256 => bool) public hyperfunds / hyperstakersprovides no way for off-chain callers to retrieve the deployed instance without reading events or off-chain indexing.
Switch tomapping(uint256 => address)(zero address = not deployed) for free discovery and contract-to-contract calls.src/Hyperfund.sol (1)
106-113:withdrawFundsshould reject zero recipient & usecallfor native transfersSending to
address(0)burns funds.
Using.transferimposes the 2300-gas stipend and may break with EIP-1884-like gas increases.- if (_token == address(0)) { - payable(_to).transfer(_amount); + require(_to != address(0), InvalidAddress()); + if (_token == address(0)) { + (bool success, ) = payable(_to).call{value: _amount}(""); + require(success, TransferFailed());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Hyperfund.sol(9 hunks)src/HyperfundFactory.sol(3 hunks)src/Hyperstaker.sol(6 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
src/Hyperfund.sol
[error] 115-119: Prettier/forge fmt formatting check failed. Function signature formatting does not match expected style. Run 'forge fmt' to fix code style issues.
🔇 Additional comments (2)
src/Hyperstaker.sol (1)
186-195: Overflow & division-by-zero risks incalculateReward
round.totalRewards * units * stakeDurationcan overflowuint256for large numbers.
Alsoround.durationmay be0(stake andsetRewardin same block) leading to division-by-zero.Consider using
mulDivfromPRBMathor an equivalent checked-mul-div helper and guardround.duration != 0.src/Hyperfund.sol (1)
224-231: Division by zero possible in_tokenAmountToUnits&_unitsToTokenAmountIf
_multiplieris0(should never happen after checks) or-1, the division path can revert or silently succeed with unintended values. Add an explicitmultiplier != 0requirement or guard upstream (seefund/redeemfixes).
Summary by CodeRabbit
New Features
Documentation