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

fix(protocol): fix OZ bump side effects #15864

Merged
merged 7 commits into from
Feb 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 20 additions & 7 deletions packages/protocol/contracts/L1/gov/TaikoGovernor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,8 @@

pragma solidity 0.8.24;

import "@openzeppelin/contracts-upgradeable/governance/GovernorUpgradeable.sol";
import
"@openzeppelin/contracts-upgradeable/governance/compatibility/GovernorCompatibilityBravoUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/governance/extensions/GovernorVotesUpgradeable.sol";
import
"@openzeppelin/contracts-upgradeable/governance/extensions/GovernorVotesQuorumFractionUpgradeable.sol";
import
Expand All @@ -26,9 +24,7 @@ import "../../common/OwnerUUPSUpgradable.sol";

contract TaikoGovernor is
OwnerUUPSUpgradable,
GovernorUpgradeable,
GovernorCompatibilityBravoUpgradeable,
GovernorVotesUpgradeable,
GovernorVotesQuorumFractionUpgradeable,
GovernorTimelockControlUpgradeable
{
Expand All @@ -42,8 +38,7 @@ contract TaikoGovernor is
initializer
{
__OwnerUUPSUpgradable_init();
__Governor_init("TaikoGovernor");
__GovernorCompatibilityBravo_init();
__Governor_init("Taiko");
__GovernorVotes_init(_token);
__GovernorVotesQuorumFraction_init(4);
__GovernorTimelockControl_init(_timelock);
Expand All @@ -68,7 +63,8 @@ contract TaikoGovernor is
override(GovernorUpgradeable, GovernorTimelockControlUpgradeable, IERC165Upgradeable)
returns (bool)
{
return super.supportsInterface(interfaceId);
return interfaceId == type(IGovernorUpgradeable).interfaceId
|| super.supportsInterface(interfaceId);
}

function state(uint256 proposalId)
Expand Down Expand Up @@ -96,6 +92,23 @@ contract TaikoGovernor is
return 1_000_000_000 ether / 10_000; // 0.01% of Taiko Token
}

/**
* @dev See {IGovernor-cancel}.
*/
function cancel(
address[] memory targets,
uint256[] memory values,
bytes[] memory calldatas,
bytes32 descriptionHash
)
public
virtual
override(IGovernorUpgradeable, GovernorUpgradeable, GovernorCompatibilityBravoUpgradeable)
returns (uint256)
{
return super.cancel(targets, values, calldatas, descriptionHash);
}

function _execute(
uint256 proposalId,
address[] memory targets,
Expand Down
2 changes: 1 addition & 1 deletion packages/protocol/test/L1/gov/TaikoGovernor.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ contract TestTaikoGovernor is TaikoL1TestBase {
assertEq(taikoGovernor.quorumDenominator(), 100, "Incorrect quorum denominator");

// GovernorUpgradeable
assertEq(taikoGovernor.name(), "TaikoGovernor", "Incorrect name");
assertEq(taikoGovernor.name(), "Taiko", "Incorrect name");
assertEq(taikoGovernor.version(), "1", "Incorrect version");

// GovernorVotesUpgradeable
Expand Down
21 changes: 9 additions & 12 deletions packages/protocol/test/common/EssentialContract.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ contract Target2 is Target1 {
}

contract TestOwnerUUPSUpgradable is TaikoTest {
/// @dev This is how we can query the admin - because from v.4.9.5 external admin() function
/// does not exist anymore.
bytes32 internal constant _ADMIN_SLOT =
0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103;

function test_essential_behind_1967_proxy() external {
bytes memory data = abi.encodeCall(Target1.init, ());
vm.startPrank(Alice);
Expand Down Expand Up @@ -80,18 +85,10 @@ contract TestOwnerUUPSUpgradable is TaikoTest {
vm.prank(Carol);
assertEq(target.owner(), Alice);

// Only Bob can call admin()
vm.prank(Bob);
assertEq(proxy.admin(), Bob);

// Other people, including Alice, cannot call admin()
vm.prank(Alice);
vm.expectRevert();
proxy.admin();

vm.prank(Carol);
vm.expectRevert();
proxy.admin();
// Admin can be queried via storage slot only - no other way.
bytes32 adminSlotValue = vm.load(address(proxy), _ADMIN_SLOT);
address admin = address(uint160(uint256(adminSlotValue)));
assertEq(admin, Bob);

// Alice can adjust();
vm.prank(Alice);
Expand Down
Loading