Feat/v0.6.0 various features - #264
Conversation
jfrancai
left a comment
jfrancai
left a comment
There was a problem hiding this comment.
Mainly naming concerns ^^
| uint128 totalAssetsLifespan; | ||
| // New variables introduce with v0.6.0 | ||
| uint256 maxCap; | ||
| bool gaveUpOperatorPrivileges; |
There was a problem hiding this comment.
I would use hasGivenUpSafePrivileges or gaveUpSafePrivileges if the length really matter more to you
| uint128 totalAssetsExpiration; | ||
| uint128 totalAssetsLifespan; | ||
| // New variables introduce with v0.6.0 | ||
| uint256 maxCap; |
There was a problem hiding this comment.
I don't see any getter for this value, I think it's important:
function maxCap() external view returns (uint256) {
return ERC7540Lib._getERC7540Storage().maxCap;
}There was a problem hiding this comment.
Test are being removed, should we reactivate them?
There was a problem hiding this comment.
no they take too long
we can reactivate them if you manage to make the execution faster
some tests run in parallel, some don't
the remaining tests cover what we need:
- a wrapped native token
- a standard ERC20 token
There was a problem hiding this comment.
we should not change v0.5.0 files
| } | ||
|
|
||
| /// @notice Deposit in a sychronous fashion into the vault. | ||
| /// @notice Deposit in a synchronous fashion into the vault. |
There was a problem hiding this comment.
we should not change v0.5.0 files
| // ## OPERATOR PRIVILEGES ## // | ||
| ////////////////////////////// | ||
|
|
||
| function giveUpOperatorPrivileges() external onlyOwner { |
There was a problem hiding this comment.
I think we should name this giveUpSafePrivileges
There was a problem hiding this comment.
I am fine with this
| ////////////////////////////// | ||
|
|
||
| function giveUpOperatorPrivileges() external onlyOwner { | ||
| _giveUpOperatorPrivileges(); |
| ) public virtual; | ||
|
|
||
| function safe() public view virtual returns (address); | ||
| function _protocolFeeReceiver() internal view virtual returns (address); |
There was a problem hiding this comment.
I'm not sure but I think we can remove this
| return address(0); | ||
| } | ||
|
|
||
| function _protocolFeeReceiver() internal view override returns (address) { |
There was a problem hiding this comment.
I'm not sure but I think we can remove this
There was a problem hiding this comment.
if solc doesn't complain, sure
| uint256 shares | ||
| ) internal virtual override { | ||
| if (caller != owner && !isOperator(owner, caller)) { | ||
| if (caller != owner && !_isOperator(owner, caller, true)) { |
- revert v0.5.0 file changes (import path and typo) - rename OperatorPrivileges to SafePrivileges for clarity - add maxCap() and gaveUpSafePrivileges() getters - update tests to reflect naming changes Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… to library - Replace onlyOperator(controller, bool) with two modifiers: - onlyOperator(controller): excludes safe as operator - onlyOperatorOrSafe(controller): includes safe as operator - Move _isOperator, _isSafeAnOperator logic to ERC7540Lib - Move _protocolFeeReceiver to RolesLib - Add isOperatorOrSafe() public view function - Remove _protocolFeeReceiver abstract function from ERC7540
address PR #264 review comments
2 participants
No description provided.