feat: added permissions for new actions and unit tests #1020
+1,214
−334
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # test/0.8.25/vaults/dashboard/dashboard.test.ts # test/0.8.25/vaults/staking-vault/stakingVault.test.ts
|
Hardhat Unit Tests Coverage SummaryDiff against masterResults for commit: e9288d6 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
There was a problem hiding this comment.
Copilot reviewed 17 out of 28 changed files in this pull request and generated no comments.
Files not reviewed (11)
- contracts/0.8.25/vaults/Dashboard.sol: Language not supported
- contracts/0.8.25/vaults/Permissions.sol: Language not supported
- contracts/0.8.25/vaults/VaultFactory.sol: Language not supported
- contracts/0.8.25/vaults/interfaces/IStakingVault.sol: Language not supported
- test/0.4.24/contracts/LidoExecutionLayerRewardsVault__MockForLidoAccounting.sol: Language not supported
- test/0.4.24/contracts/WithdrawalVault__MockForLidoAccounting.sol: Language not supported
- test/0.8.25/vaults/contracts/StakingVault__HarnessForTestUpgrade.sol: Language not supported
- test/0.8.25/vaults/dashboard/contracts/PredepositGuarantee__MockForDashboard.sol: Language not supported
- test/0.8.25/vaults/permissions/contracts/Permissions__Harness.sol: Language not supported
- test/0.8.25/vaults/permissions/contracts/PredepositGuarantee__MockPermissions.sol: Language not supported
- test/0.8.25/vaults/permissions/contracts/VaultFactory__MockPermissions.sol: Language not supported
Comments suppressed due to low confidence (3)
test/0.8.25/vaults/permissions/permissions.test.ts:110
- Using getRandomSigners to obtain signers can introduce non-determinism in tests; consider using fixed or pre-configured signers to ensure predictable outcomes.
] = await getRandomSigners(20);
lib/protocol/helpers/index.ts:17
- The removal of the getRandomSigners export from this module requires a corresponding documentation update to inform consumers of its new location in lib/account.ts.
export { getRandomSigners } from "./get-random-signers";
lib/protocol/helpers/get-random-signers.ts:1
- Ensure that no modules or tests retain stale import paths for getRandomSigners that referenced this file; all such references should now point to lib/account.ts.
Entire file removed
tamtamchik
added
solidity
There was a problem hiding this comment.
Copilot reviewed 22 out of 36 changed files in this pull request and generated no comments.
Files not reviewed (14)
- contracts/0.8.25/vaults/Dashboard.sol: Language not supported
- contracts/0.8.25/vaults/Permissions.sol: Language not supported
- contracts/0.8.25/vaults/StakingVault.sol: Language not supported
- contracts/0.8.25/vaults/VaultFactory.sol: Language not supported
- contracts/0.8.25/vaults/VaultHub.sol: Language not supported
- contracts/0.8.25/vaults/interfaces/IStakingVault.sol: Language not supported
- contracts/0.8.25/vaults/lib/PinnedBeaconUtils.sol: Language not supported
- test/0.4.24/contracts/LidoExecutionLayerRewardsVault__MockForLidoAccounting.sol: Language not supported
- test/0.4.24/contracts/WithdrawalVault__MockForLidoAccounting.sol: Language not supported
- test/0.8.25/vaults/contracts/StakingVault__HarnessForTestUpgrade.sol: Language not supported
- test/0.8.25/vaults/dashboard/contracts/ERC721__MockForDashboard.sol: Language not supported
- test/0.8.25/vaults/dashboard/contracts/PredepositGuarantee__MockForDashboard.sol: Language not supported
- test/0.8.25/vaults/dashboard/contracts/VaultFactory__MockForDashboard.sol: Language not supported
- test/0.8.25/vaults/dashboard/contracts/VaultHub__MockForDashboard.sol: Language not supported
Comments suppressed due to low confidence (2)
lib/protocol/helpers/vaults.ts:68
- Increasing the number of signers from 13 to 20 may cause unintended side effects if downstream logic expects a smaller set. Please verify that all parts of the system using these roles are compatible with the new count.
const defaultRoles = await getRandomSigners(20);
test/0.4.24/lido/lido.accounting.test.ts:153
- The helper function 'args' is used to spread parameters for collectRewardsAndProcessWithdrawals, but its definition isn’t visible in the diff. Please ensure 'args' is defined and returns the expected parameters for all test scenarios.
await expect(lido.collectRewardsAndProcessWithdrawals(...args({ elRewardsToWithdraw })))
|
Waiting for #1011 to be merged in? 🤔 |
# Conflicts: # contracts/0.8.25/vaults/Dashboard.sol # contracts/0.8.25/vaults/Permissions.sol # contracts/0.8.25/vaults/VaultFactory.sol # contracts/0.8.25/vaults/interfaces/IStakingVault.sol # lib/protocol/helpers/index.ts # lib/protocol/helpers/vaults.ts # test/0.8.25/vaults/delegation/delegation.test.ts # test/0.8.25/vaults/staking-vault/stakingVault.test.ts # test/0.8.25/vaults/vaultFactory.test.ts # test/0.8.25/vaults/vaulthub/vaulthub.detach.test.ts # test/0.8.9/accounting.handleOracleReport.test.ts # test/integration/vaults/happy-path.integration.ts # test/integration/vaults/roles.integration.ts
| */ | ||
| bytes32 public constant ASSET_RECOVERY_ROLE = keccak256("vaults.Permissions.AssetRecovery"); | ||
| bytes32 public constant LIDO_VAULTHUB_AUTHORIZATION_ROLE = |
There was a problem hiding this comment.
Suggested change
| bytes32 public constant LIDO_VAULTHUB_AUTHORIZATION_ROLE = | |
| bytes32 public constant LIDO_VAULTHUB_AUTHORIZE_ROLE = |
There was a problem hiding this comment.
Or even AUTHORIZE_LIDO_VAULTHUB_ROLE?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request includes several changes across multiple files to enhance the vault system's functionality and permissions management. The most important changes include adding new roles and permissions, updating functions for managing the staking vault, and refactoring existing code for clarity and consistency.
Enhancements to Permissions and Roles:
contracts/0.8.25/vaults/Permissions.sol: Added new roles includingLIDO_VAULTHUB_AUTHORIZATION_ROLE,OSSIFY_ROLE,SET_DEPOSITOR_ROLE, andRESET_LOCKED_ROLEto manage various staking vault operations.Updates to Staking Vault Management:
contracts/0.8.25/vaults/Dashboard.sol: Introduced functions likeauthorizeLidoVaultHub,ossifyStakingVault,setDepositor, andresetLockedto manage the staking vault's state and permissions.contracts/0.8.25/vaults/StakingVault.sol: Refactored functions to use the termossifiedinstead ofisOssifiedfor consistency and added new events and checks.Code Refactoring and Consistency:
contracts/0.8.25/vaults/lib/PinnedBeaconUtils.sol: Updated function names fromisOssifiedtoossifiedfor consistency across the codebase.contracts/0.8.25/vaults/VaultFactory.sol: Added new role assignments forpdgWithdrawers,lidoVaultHubAuthorizers,ossifiers,depositorSetters, andlockedResetters.Utility Functions and Helper Methods:
lib/account.ts: Added a new functiongetRandomSignersto generate random signers with a specified balance for testing purposes.lib/protocol/helpers/vaults.ts: Integrated thegetRandomSignersfunction and expanded theVaultRolestype to include new roles.These changes collectively enhance the functionality, security, and maintainability of the vault system by introducing new roles, improving permission management, and ensuring consistency across the codebase.