Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: added reinitializer and storage gap to PluginRepo and moved events to IPluginRepo #584

Merged
merged 13 commits into from
May 10, 2024
Merged
17 changes: 17 additions & 0 deletions packages/contracts/src/framework/plugin/repo/IPluginRepo.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,21 @@ interface IPluginRepo {
bytes calldata _buildMetadata,
bytes calldata _releaseMetadata
) external;

/// @notice Thrown if the same plugin setup exists in previous releases.
josemarinas marked this conversation as resolved.
Show resolved Hide resolved
/// @param release The release number.
/// @param build The build number.
/// @param pluginSetup The address of the plugin setup contract.
/// @param buildMetadata The build metadata URI.
event VersionCreated(
uint8 release,
uint16 build,
address indexed pluginSetup,
bytes buildMetadata
);

/// @notice Thrown when a release's metadata was updated.
josemarinas marked this conversation as resolved.
Show resolved Hide resolved
/// @param release The release number.
/// @param releaseMetadata The release metadata URI.
event ReleaseMetadataUpdated(uint8 release, bytes releaseMetadata);
}
39 changes: 21 additions & 18 deletions packages/contracts/src/framework/plugin/repo/PluginRepo.sol
Original file line number Diff line number Diff line change
Expand Up @@ -95,23 +95,8 @@ contract PluginRepo is

/// @notice Thrown if release does not exist.
error ReleaseDoesNotExist();

/// @notice Thrown if the same plugin setup exists in previous releases.
/// @param release The release number.
/// @param build The build number.
/// @param pluginSetup The address of the plugin setup contract.
/// @param buildMetadata The build metadata URI.
event VersionCreated(
uint8 release,
uint16 build,
address indexed pluginSetup,
bytes buildMetadata
);

/// @notice Thrown when a release's metadata was updated.
/// @param release The release number.
/// @param releaseMetadata The release metadata URI.
event ReleaseMetadataUpdated(uint8 release, bytes releaseMetadata);
/// @notice Thrown if an upgrade is not supported from a specific protocol version .
error ProtocolVersionUpgradeNotSupported(uint8[3] protocolVersion);
heueristik marked this conversation as resolved.
Show resolved Hide resolved

/// @dev Used to disallow initializing the implementation contract by an attacker for extra safety.
constructor() {
Expand All @@ -122,13 +107,28 @@ contract PluginRepo is
/// - initializing the permission manager
/// - granting the `MAINTAINER_PERMISSION_ID` permission to the initial owner.
/// @dev This method is required to support [ERC-1822](https://eips.ethereum.org/EIPS/eip-1822).
function initialize(address initialOwner) external initializer {
function initialize(address initialOwner) external reinitializer(2) {
josemarinas marked this conversation as resolved.
Show resolved Hide resolved
__PermissionManager_init(initialOwner);

_grant(address(this), initialOwner, MAINTAINER_PERMISSION_ID);
_grant(address(this), initialOwner, UPGRADE_REPO_PERMISSION_ID);
}

/// @notice Initializes the pluginRepo after an upgrade from a previous protocol version.
/// @param _previousProtocolVersion The semantic protocol version number of the previous DAO implementation contract this upgrade is transitioning from.
/// @param _initData The initialization data to be passed to via `upgradeToAndCall` (see [ERC-1967](https://docs.openzeppelin.com/contracts/4.x/api/proxy#ERC1967Upgrade)).
function initializeFrom(
uint8[3] calldata _previousProtocolVersion,
josemarinas marked this conversation as resolved.
Show resolved Hide resolved
bytes calldata _initData
) external reinitializer(2) {
_initData; // Silences the unused function parameter warning.

// Check that the contract is not upgrading from a different major release.
if (_previousProtocolVersion[0] != 1) {
revert ProtocolVersionUpgradeNotSupported(_previousProtocolVersion);
}
josemarinas marked this conversation as resolved.
Show resolved Hide resolved
josemarinas marked this conversation as resolved.
Show resolved Hide resolved
}

/// @inheritdoc IPluginRepo
function createVersion(
uint8 _release,
Expand Down Expand Up @@ -272,4 +272,7 @@ contract PluginRepo is
_interfaceId == type(IProtocolVersion).interfaceId ||
super.supportsInterface(_interfaceId);
}

/// @notice This empty reserved space is put in place to allow future versions to add new variables without shifting down storage in the inheritance chain (see [OpenZeppelin's guide about storage gaps](https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps)).
uint256[46] private __gap;
heueristik marked this conversation as resolved.
Show resolved Hide resolved
}
2 changes: 1 addition & 1 deletion packages/contracts/test/framework/plugin/plugin-repo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ describe('PluginRepo', function () {
).to.be.revertedWithCustomError(pluginRepo, 'EmptyReleaseMetadata');
});

it('successfuly updates metadata for the release that already exists', async () => {
it('successfully updates metadata for the release that already exists and emits the "ReleaseMetadataUpdated" event', async () => {
josemarinas marked this conversation as resolved.
Show resolved Hide resolved
await pluginRepo.createVersion(
1,
pluginSetupMock.address,
Expand Down
Loading