Skip to content

Commit

Permalink
feat: safer governance implementation initialization
Browse files Browse the repository at this point in the history
- init staking proxy to address(1) in the constructor to prevent direct calls
- fix virtual modifiers definition as hardhat coverage still not capable of reading it correctly.
- add comment for help/safety for future upgrades
  • Loading branch information
gabririgo committed Jan 20, 2023
1 parent aceba44 commit a06b0a5
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 5 deletions.
8 changes: 5 additions & 3 deletions contracts/governance/RigoblockGovernance.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,11 @@ contract RigoblockGovernance is

/// @notice Constructor has no inputs to guarantee same deterministic address.
/// @dev Different parameters on each would result in different implementation address.
/// @dev Setting staking proxy effectively locks direct calls to this contract.
constructor(address initializer) MixinImmutables(address(this), initializer) {
/// @dev Initializing to address(1) effectively locks direct calls to implementation.
constructor(address initializer)
MixinImmutables(address(this), initializer)
MixinInitializer(address(1))
{
paramsWrapper().treasuryParameters.proposalThreshold = type(uint256).max;
stakingProxy().value = address(0);
}
}
10 changes: 9 additions & 1 deletion contracts/governance/mixins/MixinInitializer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,15 @@ import "./MixinAbstract.sol";
import "./MixinStorage.sol";

abstract contract MixinInitializer is MixinStorage, MixinAbstract {
modifier onlyDelegatecall() virtual;
// This modifier is not necessary as implementation initialization is prevented
// by asserting staking proxy null inside the method, but we keep it for future
// upgrades extra security to prevent accidental initialization.
modifier onlyDelegatecall() virtual {_;}

/// @notice We lock future calls to this implementation.
constructor(address stakingProxy_) {
stakingProxy().value = stakingProxy_;
}

/// @inheritdoc IGovernanceInitializer
function initializeGovernance(
Expand Down
2 changes: 1 addition & 1 deletion contracts/governance/mixins/MixinUpgrade.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import "../../utils/storageSlot/StorageSlot.sol";
import "./MixinStorage.sol"; // storage inherits from interface which declares events

abstract contract MixinUpgrade is MixinStorage {
modifier onlyDelegatecall() virtual;
modifier onlyDelegatecall() virtual {_;}

/// @inheritdoc IGovernanceUpgrade
function upgradeImplementation(address newImplementation) external onlyDelegatecall override {
Expand Down

0 comments on commit a06b0a5

Please sign in to comment.