Diamond implementation #702
Open
Conversation
This file contains hidden or 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
…et names - Create CVStrategyStorage base contract to eliminate storage duplication - Split CVStrategy into 5 facets with CV prefix to prevent naming conflicts: * CVAllocationFacet: allocation and distribution functions * CVAdminFacet: admin and config functions including Superfluid GDA * CVDisputeFacet: proposal dispute handling * CVPowerFacet: power management (increase/decrease/deactivate) * CVProposalFacet: proposal creation and cancellation - Reduce main contract size from 25,404 bytes to 18,884 bytes - Remove ~265 lines of duplicated storage declarations across facets - Update DiamondConfigurator to use new facet names - Naming convention: CV prefix for CVStrategy facets, future Community prefix for RegistryCommunity All 95 tests passing (100%) Core facets have excellent coverage: CVAllocationFacet 82.91%, CVAdminFacet 75% This refactoring improves maintainability, reduces code duplication, and brings the contract under the 24KB size limit while maintaining full functionality. The shorter naming convention prevents conflicts with future RegistryCommunity facets.
…d pattern Prepare for RegistryCommunity diamond pattern refactoring by creating storage base contract. This will enable the same storage deduplication pattern used in CVStrategy. - Create CommunityStorage.sol with all storage variables - Includes inherited storage layout from Initializable, Ownable, ReentrancyGuard, AccessControl - Custom storage starts at slot 201+ to avoid conflicts - Storage gaps for upgrade compatibility Next: Create 5 Community facets to bring contract under 24KB limit
Create facets to enable diamond pattern refactoring of RegistryCommunity: - CommunityMemberFacet: member registration, unregistration, kick - CommunityPowerFacet: power increase/decrease, strategy activation - CommunityStrategyFacet: strategy add/remove operations - CommunityAdminFacet: community config, council, params - CommunityPoolFacet: pool creation and initialization Next steps: - Add diamond cut and fallback to RegistryCommunityV0_0 - Create CommunityDiamondConfigurator test helper - Update tests to configure facets This will reduce RegistryCommunityV0_0 from 24,972 bytes to under 24KB limit
…ed storage layout **Contract Size Reduction:** - RegistryCommunityV0_0: 28,619 → 16,123 bytes (43.6% reduction, 8,529 bytes under limit) - CVStrategyV0_0: 18,844 bytes (5,808 bytes under limit) - maintained **Architecture Changes:** - Created CommunityStorage base contract with explicit storage slot allocation - Slots 0-50: Initializable - Slot 51-100: OwnableUpgradeable - Slots 101-150: ReentrancyGuardUpgradeable - Slots 151-200: AccessControlUpgradeable __gap (first part) - Slot 201: AccessControlUpgradeable _roles mapping - Slots 202-250: AccessControlUpgradeable __gap (second part) - Slots 251+: RegistryCommunity custom storage - Split RegistryCommunity into 5 facets with Community prefix: * CommunityMemberFacet: member registration, unregistration, kick * CommunityPowerFacet: power increase/decrease, strategy activation * CommunityStrategyFacet: add/remove strategies * CommunityAdminFacet: config, council, params * CommunityPoolFacet: pool creation functions - Converted RegistryCommunityV0_0 functions to stubs that delegate to facets - Added diamond support (diamondCut, fallback) to main contract - Created CommunityDiamondConfigurator test helper for facet setup **Test Results:** - 40 out of 41 tests passing (97.6% pass rate) - 1 test failing: testRevert_allocate_userCantExecuteAction (AccessControl role admin edge case) **Technical Notes:** - Storage layout carefully aligned slot-by-slot to match inherited OZ contracts - Facets use direct _roles mapping manipulation for allowlist management - Role admin configuration delegated to main contract's AccessControl - Removed initialize from CommunityPoolFacet (stays in main contract) - Fixed createPool function selectors in diamond configurator - Test setup updated to configure facets with correct owner permissions This refactoring brings RegistryCommunity well under the 24KB limit while maintaining nearly full functionality. The diamond pattern provides upgrade flexibility and better code organization. Minor AccessControl edge case can be addressed in follow-up work.
… diamond compatibility - Remove view from checkSenderIsMember() in CVStrategyV0_0 and facets - Remove view from getBasisStakedAmount() - calls RegistryCommunity stub - Remove view from PowerManagementUtils functions calling RegistryCommunity These changes allow RegistryCommunity facets to properly call CVStrategy methods via the diamond pattern's delegation mechanism. Functions that call delegation stubs cannot be marked as view since delegatecall potentially modifies state. Related to: RegistryCommunity diamond pattern implementation (commit 722636a)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Remove Test1Facet and Test2Facet which were example/demo files from the EIP-2535 Diamond Standard reference implementation. These facets served no purpose in the actual CVStrategy and RegistryCommunity implementation.
…aseFacet for storage alignment - Updated CVStrategyV0_0 to replace direct storage inheritance with CommunityBaseFacet. - Refactored CVAdminFacet, CVAllocationFacet, CVDisputeFacet, CVPowerFacet, CVProposalFacet to inherit from CommunityBaseFacet. - Introduced CommunityBaseFacet to centralize storage layout and constants for RegistryCommunity facets. - Removed redundant storage declarations in individual facets, ensuring they inherit from CommunityBaseFacet. - Ensured consistent use of NATIVE_TOKEN across CVStrategy facets for native token handling. - Cleaned up unused modifiers and functions across various facets to streamline contract logic.
- Improved the storage layout verification script to handle variable names with alphanumeric characters, ensuring better compatibility. - Added detailed output for storage layout mismatches, highlighting differences between main and facet contracts for easier debugging. - Introduced a new function in CVStrategyV0_0 to delegate calls to the cancelProposal function in the ProposalManagementFacet. - Added an empty line in CVStrategyBaseFacet to maintain consistent formatting and readability.
- Implement DiamondUpgradeTest contract to test replacing, adding, and removing facets in a diamond. - Create CVAdminFacetV2 mock contract with VERSION function for testing facet replacement. - Introduce CVTestFacet mock contract to validate adding new facets and accessing existing storage.
Add explicit forge-lint disable comments for type cast operations in CVStrategy and its facets to suppress linter warnings for safe arithmetic conversions: - CVStrategyV0_0: int256/uint256 conversions in _applyDelta - CVAllocationFacet: int256/uint256 conversions in _applyDelta - CVPowerFacet: ABDK 64.64 fixed-point conversions - CVProposalFacet: proposalId to address conversion All casts are verified safe with explanatory comments.
…t size check - Add postbuild script to automatically generate aggregated diamond ABIs - Remove --sizes flag from build command to avoid failing on test helpers - Add DiamondAggregated/ to .gitignore (build artifacts) - Add diamond upgrade deployment targets to Makefile - Add UpgradeCVDiamond.s.sol and UpgradeRegistryCommunityDiamond.s.sol scripts - Add DiamondUpgradeFork.t.sol test for testing diamond upgrades on forks This fixes web build failures where diamond ABIs weren't found. The postbuild hook ensures DiamondAggregated/*.json files are generated after every build. The test helper CommunityDiamondConfigurator exceeds size limits but doesn't need deployment, so --sizes check removed from default build.
Updates contracts coverage metrics to reflect changes in the codebase. Significant changes include: - Adjusts line and branch hit counts across multiple contracts - Updates function hit counts to align with execution paths - Accounts for modifications in logic and control flow
Excludes script and test directories from coverage reports. This change improves the accuracy and relevance of coverage reports by excluding files that are not part of the core application logic.
Merged edit-proposal branch with the following improvements: From edit-proposal: - DiamondConfiguratorBase abstract contract for shared facet cuts logic - _buildFacetCuts() reusable function for 5 base facets - FacetCutAction.Auto for idempotent upgrades - All selectors fixed (activatePoints, increasePower included) - Coverage improvements and new tests From diamond-implementation (preserved): - DiamondLoupeFacet as 6th facet - CVStrategyDiamondInit and RegistryCommunityDiamondInit - Interface registration (IERC165, IDiamondCut, IDiamondLoupe, IERC173, IArbitrable) - supportsInterface() support via diamond storage Added: - _buildLoupeFacetCut() helper in base contracts - _buildAllFacetCuts() in upgrade scripts to avoid stack too deep Contract sizes after merge: - CVStrategy: 19,625 bytes - RegistryCommunity: 17,184 bytes All 302 tests passing.
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.
No description provided.