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);
}
38 changes: 21 additions & 17 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 @@ -129,6 +114,22 @@ contract PluginRepo is
_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)).
/// @dev This function is a placeholder until we require reinitialization.
function initializeFrom(
uint8[3] calldata _previousProtocolVersion,
josemarinas marked this conversation as resolved.
Show resolved Hide resolved
bytes calldata _initData
) external reinitializer(2) {
// Silences the unused function parameter warning.
_previousProtocolVersion;
_initData;

josemarinas marked this conversation as resolved.
Show resolved Hide resolved
// Revert because this is a placeholder until this contract requires reinitialization.
revert();
}

/// @inheritdoc IPluginRepo
function createVersion(
uint8 _release,
Expand Down Expand Up @@ -272,4 +273,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('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