Skip to content

Commit 8f74015

Browse files
authored
Merge pull request #519 from bane-labs/remove-proxy-check
contracts: remove proxy checks and upgrade hardhat
2 parents 18f8547 + e5db0bf commit 8f74015

17 files changed

+236
-343
lines changed

contracts/hardhat.config.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,11 @@
11
import type { HardhatUserConfig } from "hardhat/config";
22

3+
import hardhatEthers from "@nomicfoundation/hardhat-ethers";
4+
import hardhatNetworkHelpers from "@nomicfoundation/hardhat-network-helpers";
35
import hardhatToolboxMochaEthersPlugin from "@nomicfoundation/hardhat-toolbox-mocha-ethers";
46

57
const config: HardhatUserConfig = {
6-
/*
7-
* In Hardhat 3, plugins are defined as part of the Hardhat config instead of
8-
* being based on the side-effect of imports.
9-
*
10-
* Note: A `hardhat-toolbox` like plugin for Hardhat 3 hasn't been defined yet,
11-
* so this list is larger than what you would normally have.
12-
*/
13-
plugins: [hardhatToolboxMochaEthersPlugin],
8+
plugins: [hardhatEthers, hardhatNetworkHelpers, hardhatToolboxMochaEthersPlugin],
149
solidity: {
1510
profiles: {
1611
default: {

contracts/package.json

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,21 @@
11
{
22
"name": "neox-governance",
3-
"type": "module",
43
"version": "1.0.0",
4+
"type": "module",
55
"devDependencies": {
6-
"@nomicfoundation/hardhat-ethers": "^4.0.0-next.21",
7-
"@nomicfoundation/hardhat-ignition": "^3.0.0-next.21",
8-
"@nomicfoundation/hardhat-toolbox-mocha-ethers": "^3.0.0-next.21",
6+
"@nomicfoundation/hardhat-ethers": "^4.0.2",
7+
"@nomicfoundation/hardhat-ignition": "^3.0.3",
8+
"@nomicfoundation/hardhat-network-helpers": "^3.0.1",
9+
"@nomicfoundation/hardhat-toolbox-mocha-ethers": "^3.0.0",
910
"@types/chai": "^4.3.20",
1011
"@types/chai-as-promised": "^8.0.2",
1112
"@types/mocha": "^10.0.10",
12-
"@types/node": "^22.16.0",
13-
"chai": "^5.2.0",
13+
"@types/node": "^22.18.10",
14+
"chai": "^5.3.3",
1415
"ethers": "^6.15.0",
1516
"forge-std": "github:foundry-rs/forge-std#v1.9.4",
16-
"hardhat": "^3.0.0-next.21",
17-
"mocha": "^11.7.1",
17+
"hardhat": "^3.0.7",
18+
"mocha": "^11.7.4",
1819
"typescript": "~5.8.0"
1920
},
2021
"dependencies": {

contracts/solidity/CommitteeMultiSig.sol

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,32 +2,10 @@
22
pragma solidity ^0.8.25;
33

44
import {GovernanceVote} from "./base/GovernanceVote.sol";
5-
import {ERC1967Utils, GovProxyUpgradeable} from "./base/GovProxyUpgradeable.sol";
5+
import {GovProxyUpgradeable} from "./base/GovProxyUpgradeable.sol";
66
import {Address} from "@openzeppelin/contracts/utils/Address.sol";
77

88
contract CommitteeMultiSig is GovernanceVote, GovProxyUpgradeable {
9-
address public constant SELF = 0x1212100000000000000000000000000000000007;
10-
11-
// Only for precompiled uups implementation in genesis file, need to be removed when upgrading the contract.
12-
// This override is added because "immutable __self" in UUPSUpgradeable is not avaliable in precompiled contract.
13-
function _checkProxy() internal view virtual override {
14-
if (
15-
address(this) == SELF || // Must be called through delegatecall
16-
ERC1967Utils.getImplementation() != SELF // Must be called through an active proxy
17-
) {
18-
revert UUPSUnauthorizedCallContext();
19-
}
20-
}
21-
22-
// Only for precompiled uups implementation in genesis file, need to be removed when upgrading the contract.
23-
// This override is added because "immutable __self" in UUPSUpgradeable is not avaliable in precompiled contract.
24-
function _checkNotDelegated() internal view virtual override {
25-
if (address(this) != SELF) {
26-
// Must not be called through delegatecall
27-
revert UUPSUnauthorizedCallContext();
28-
}
29-
}
30-
319
// Execute an operation that calls a function on the `target` contract
3210
function execute(
3311
address target,

contracts/solidity/GovReward.sol

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,9 @@ import {Errors} from "./libraries/Errors.sol";
55
import {Bytes} from "./libraries/Bytes.sol";
66
import {IGovReward} from "./interfaces/IGovReward.sol";
77
import {IGovernance} from "./interfaces/IGovernance.sol";
8-
import {ERC1967Utils, GovProxyUpgradeable} from "./base/GovProxyUpgradeable.sol";
8+
import {GovProxyUpgradeable} from "./base/GovProxyUpgradeable.sol";
99

1010
contract GovReward is IGovReward, GovProxyUpgradeable {
11-
address public constant SELF = 0x1212100000000000000000000000000000000003;
1211
// governance contact
1312
address public constant GOV = 0x1212000000000000000000000000000000000001;
1413

@@ -36,26 +35,6 @@ contract GovReward is IGovReward, GovProxyUpgradeable {
3635
_;
3736
}
3837

39-
// Only for precompiled uups implementation in genesis file, need to be removed when upgrading the contract.
40-
// This override is added because "immutable __self" in UUPSUpgradeable is not avaliable in precompiled contract.
41-
function _checkProxy() internal view virtual override {
42-
if (
43-
address(this) == SELF || // Must be called through delegatecall
44-
ERC1967Utils.getImplementation() != SELF // Must be called through an active proxy
45-
) {
46-
revert UUPSUnauthorizedCallContext();
47-
}
48-
}
49-
50-
// Only for precompiled uups implementation in genesis file, need to be removed when upgrading the contract.
51-
// This override is added because "immutable __self" in UUPSUpgradeable is not avaliable in precompiled contract.
52-
function _checkNotDelegated() internal view virtual override {
53-
if (address(this) != SELF) {
54-
// Must not be called through delegatecall
55-
revert UUPSUnauthorizedCallContext();
56-
}
57-
}
58-
5938
function getMiners() external view override returns (address[] memory) {
6039
return IGovernance(GOV).getCurrentConsensus();
6140
}

contracts/solidity/Governance.sol

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,13 @@ import {IGovReward} from "./interfaces/IGovReward.sol";
66
import {IKeyManagement} from "./interfaces/IKeyManagement.sol";
77
import {IGovernance} from "./interfaces/IGovernance.sol";
88
import {IPolicy} from "./interfaces/IPolicy.sol";
9-
import {ERC1967Utils, GovProxyUpgradeable} from "./base/GovProxyUpgradeable.sol";
9+
import {GovProxyUpgradeable} from "./base/GovProxyUpgradeable.sol";
1010
import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
1111
import {ReentrancyGuard} from "@openzeppelin/contracts/utils/ReentrancyGuard.sol";
1212

1313
contract Governance is IGovernance, ReentrancyGuard, GovProxyUpgradeable {
1414
using EnumerableSet for EnumerableSet.AddressSet;
1515

16-
address public constant SELF = 0x1212100000000000000000000000000000000001;
1716
// Policy contract
1817
address public constant POLICY = 0x1212000000000000000000000000000000000002;
1918
// GovReward contract
@@ -76,26 +75,6 @@ contract Governance is IGovernance, ReentrancyGuard, GovProxyUpgradeable {
7675
// the pending group of block validators
7776
address[] public pendingConsensus;
7877

79-
// Only for precompiled uups implementation in genesis file, need to be removed when upgrading the contract.
80-
// This override is added because "immutable __self" in UUPSUpgradeable is not avaliable in precompiled contract.
81-
function _checkProxy() internal view virtual override {
82-
if (
83-
address(this) == SELF || // Must be called through delegatecall
84-
ERC1967Utils.getImplementation() != SELF // Must be called through an active proxy
85-
) {
86-
revert UUPSUnauthorizedCallContext();
87-
}
88-
}
89-
90-
// Only for precompiled uups implementation in genesis file, need to be removed when upgrading the contract.
91-
// This override is added because "immutable __self" in UUPSUpgradeable is not avaliable in precompiled contract.
92-
function _checkNotDelegated() internal view virtual override {
93-
if (address(this) != SELF) {
94-
// Must not be called through delegatecall
95-
revert UUPSUnauthorizedCallContext();
96-
}
97-
}
98-
9978
receive() external payable nonReentrant {
10079
if (msg.sender != GOV_REWARD) revert Errors.SideCallNotAllowed();
10180
address[] memory validators = currentConsensus;

contracts/solidity/KeyManagementV0.sol

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,9 @@ import {BLS12381} from "./libraries/BLS12381.sol";
66
import {IGovernance} from "./interfaces/IGovernance.sol";
77
import {IKeyManagement} from "./interfaces/IKeyManagement.sol";
88
import {IZKDKGV0} from "./interfaces/IZKDKGV0.sol";
9-
import {ERC1967Utils, GovProxyUpgradeable} from "./base/GovProxyUpgradeable.sol";
9+
import {GovProxyUpgradeable} from "./base/GovProxyUpgradeable.sol";
1010

1111
contract KeyManagementV0 is GovProxyUpgradeable, IKeyManagement, IZKDKGV0 {
12-
address public constant SELF = 0x1212100000000000000000000000000000000008;
1312
// governance contact
1413
address public constant GOV = 0x1212000000000000000000000000000000000001;
1514
address public constant SYS_CALL =
@@ -43,26 +42,6 @@ contract KeyManagementV0 is GovProxyUpgradeable, IKeyManagement, IZKDKGV0 {
4342
// ref https://github.com/bane-labs/go-ethereum/blob/a07310bd9a3a117ae0876ad69bbe8b6ed624aaa5/core/antimev/util.go#L27
4443
mapping(uint => bytes) public aggregatedCommitments;
4544

46-
// Only for precompiled uups implementation in genesis file, need to be removed when upgrading the contract.
47-
// This override is added because "immutable __self" in UUPSUpgradeable is not avaliable in precompiled contract.
48-
function _checkProxy() internal view virtual override {
49-
if (
50-
address(this) == SELF || // Must be called through delegatecall
51-
ERC1967Utils.getImplementation() != SELF // Must be called through an active proxy
52-
) {
53-
revert UUPSUnauthorizedCallContext();
54-
}
55-
}
56-
57-
// Only for precompiled uups implementation in genesis file, need to be removed when upgrading the contract.
58-
// This override is added because "immutable __self" in UUPSUpgradeable is not avaliable in precompiled contract.
59-
function _checkNotDelegated() internal view virtual override {
60-
if (address(this) != SELF) {
61-
// Must not be called through delegatecall
62-
revert UUPSUnauthorizedCallContext();
63-
}
64-
}
65-
6645
function registerMessageKey(
6746
address candidate,
6847
bytes calldata pubkey

contracts/solidity/KeyManagementV1.sol

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,9 @@ import {SevenMessageVerifier} from "./libraries/SevenMessageVerifier.sol";
99
import {IGovernance} from "./interfaces/IGovernance.sol";
1010
import {IKeyManagement} from "./interfaces/IKeyManagement.sol";
1111
import {IZKDKGV1} from "./interfaces/IZKDKGV1.sol";
12-
import {ERC1967Utils, GovProxyUpgradeable} from "./base/GovProxyUpgradeable.sol";
12+
import {GovProxyUpgradeable} from "./base/GovProxyUpgradeable.sol";
1313

1414
contract KeyManagementV1 is GovProxyUpgradeable, IKeyManagement, IZKDKGV1 {
15-
address public constant SELF = 0x1212100000000000000000000000000000000008;
1615
// governance contact
1716
address public constant GOV = 0x1212000000000000000000000000000000000001;
1817
address public constant SYS_CALL =
@@ -48,26 +47,6 @@ contract KeyManagementV1 is GovProxyUpgradeable, IKeyManagement, IZKDKGV1 {
4847
// hash=>used, this is used to prevent reusing and uploading the same public input
4948
mapping(bytes32 => bool) public isPubHashUsed;
5049

51-
// Only for precompiled uups implementation in genesis file, need to be removed when upgrading the contract.
52-
// This override is added because "immutable __self" in UUPSUpgradeable is not avaliable in precompiled contract.
53-
function _checkProxy() internal view virtual override {
54-
if (
55-
address(this) == SELF || // Must be called through delegatecall
56-
ERC1967Utils.getImplementation() != SELF // Must be called through an active proxy
57-
) {
58-
revert UUPSUnauthorizedCallContext();
59-
}
60-
}
61-
62-
// Only for precompiled uups implementation in genesis file, need to be removed when upgrading the contract.
63-
// This override is added because "immutable __self" in UUPSUpgradeable is not avaliable in precompiled contract.
64-
function _checkNotDelegated() internal view virtual override {
65-
if (address(this) != SELF) {
66-
// Must not be called through delegatecall
67-
revert UUPSUnauthorizedCallContext();
68-
}
69-
}
70-
7150
function registerMessageKey(
7251
address candidate,
7352
bytes calldata pubkey

contracts/solidity/Policy.sol

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,9 @@ import {Errors} from "./libraries/Errors.sol";
55
import {IGovernance} from "./interfaces/IGovernance.sol";
66
import {IPolicy} from "./interfaces/IPolicy.sol";
77
import {GovernanceVote} from "./base/GovernanceVote.sol";
8-
import {ERC1967Utils, GovProxyUpgradeable} from "./base/GovProxyUpgradeable.sol";
8+
import {GovProxyUpgradeable} from "./base/GovProxyUpgradeable.sol";
99

1010
contract Policy is IPolicy, GovernanceVote, GovProxyUpgradeable {
11-
address public constant SELF = 0x1212100000000000000000000000000000000002;
1211
// governance contact
1312
address public constant GOV = 0x1212000000000000000000000000000000000001;
1413
uint256 public constant DEFAULT_CANDIDATE_LIMIT = 2000;
@@ -21,26 +20,6 @@ contract Policy is IPolicy, GovernanceVote, GovProxyUpgradeable {
2120
uint256 public maxEnvelopesPerBlock;
2221
uint256 public maxEnvelopeGasLimit;
2322

24-
// Only for precompiled uups implementation in genesis file, need to be removed when upgrading the contract.
25-
// This override is added because "immutable __self" in UUPSUpgradeable is not avaliable in precompiled contract.
26-
function _checkProxy() internal view virtual override {
27-
if (
28-
address(this) == SELF || // Must be called through delegatecall
29-
ERC1967Utils.getImplementation() != SELF // Must be called through an active proxy
30-
) {
31-
revert UUPSUnauthorizedCallContext();
32-
}
33-
}
34-
35-
// Only for precompiled uups implementation in genesis file, need to be removed when upgrading the contract.
36-
// This override is added because "immutable __self" in UUPSUpgradeable is not avaliable in precompiled contract.
37-
function _checkNotDelegated() internal view virtual override {
38-
if (address(this) != SELF) {
39-
// Must not be called through delegatecall
40-
revert UUPSUnauthorizedCallContext();
41-
}
42-
}
43-
4423
function addBlackList(
4524
address _addr
4625
)
Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,51 @@
11
import { expect } from "chai";
22
import { ERRORS } from "./helpers/errors.js";
3-
import { ethers, allocGenesis } from "./helpers/setup.js";
3+
import { ethers, networkHelpers, allocGenesis } from "./helpers/setup.js";
44

55
describe("CommitteeMultiSig", function () {
66

7-
let signers: any;
7+
let MultiSig: any, MockCaller: any;
8+
let signers: any, snapshot: any;
89

9-
beforeEach(async function () {
10+
before(async function () {
1011
signers = await ethers.getSigners();
1112
await allocGenesis();
13+
MultiSig = await ethers.deployContract("CommitteeMultiSig");
14+
MockCaller = await ethers.deployContract("MockMultiSig");
15+
snapshot = await networkHelpers.takeSnapshot();
1216
});
1317

14-
describe("execute", function () {
15-
let MultiSig: any, Mock: any;
16-
17-
beforeEach(async function () {
18-
MultiSig = await ethers.deployContract("CommitteeMultiSig");
19-
Mock = await ethers.deployContract("MockMultiSig");
20-
});
18+
afterEach(async function () {
19+
await snapshot.restore();
20+
});
2121

22+
describe("execute", function () {
2223
it("Should revert if the sender is not a miner", async function () {
2324
await expect(
24-
MultiSig.connect(signers[7]).execute(Mock.target, "0xa1b2ca7d0000000000000000000000000000000000000000000000000000000000000001")
25+
MultiSig.connect(signers[7]).execute(MockCaller.target, "0xa1b2ca7d0000000000000000000000000000000000000000000000000000000000000001")
2526
).to.be.revertedWithCustomError(MultiSig, ERRORS.NOT_MINER);
2627
});
2728

2829
it("Should not execute method when threshold is not met", async function () {
2930
await expect(
30-
MultiSig.connect(signers[0]).execute(Mock.target, "0xa1b2ca7d0000000000000000000000000000000000000000000000000000000000000001")
31-
).not.to.be.reverted(ethers);
31+
MultiSig.connect(signers[0]).execute(MockCaller.target, "0xa1b2ca7d0000000000000000000000000000000000000000000000000000000000000001")
32+
).not.to.be.revert(ethers);
3233

33-
expect(await Mock.v()).to.eq(0);
34+
expect(await MockCaller.v()).to.eq(0);
3435
});
3536

3637
it("Should execute method when threshold is met", async function () {
3738
for (let i = 0; i < 3; i++) {
3839
await expect(
39-
MultiSig.connect(signers[i]).execute(Mock.target, "0xa1b2ca7d0000000000000000000000000000000000000000000000000000000000000001")
40-
).not.to.be.reverted(ethers);
40+
MultiSig.connect(signers[i]).execute(MockCaller.target, "0xa1b2ca7d0000000000000000000000000000000000000000000000000000000000000001")
41+
).not.to.be.revert(ethers);
4142
}
42-
expect(await Mock.v()).to.eq(0);
43+
expect(await MockCaller.v()).to.eq(0);
4344

4445
await expect(
45-
MultiSig.connect(signers[3]).execute(Mock.target, "0xa1b2ca7d0000000000000000000000000000000000000000000000000000000000000001")
46-
).not.to.be.reverted(ethers);
47-
expect(await Mock.v()).to.eq(1);
46+
MultiSig.connect(signers[3]).execute(MockCaller.target, "0xa1b2ca7d0000000000000000000000000000000000000000000000000000000000000001")
47+
).not.to.be.revert(ethers);
48+
expect(await MockCaller.v()).to.eq(1);
4849
});
4950
});
50-
});
51+
});

contracts/test/GovProxyUpgradeable.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,27 +4,32 @@ import { expect } from "chai";
44
const { ethers } = await network.connect();
55

66
describe("GovProxyUpgradeable", function () {
7+
8+
let Mock: any;
9+
10+
before(async function () {
11+
Mock = await ethers.deployContract("MockGovProxyUpgradeable");
12+
});
13+
714
it("Should prevent implementation contract from initialization", async function () {
8-
const mockGovProxyUpgradeable = await ethers.deployContract("MockGovProxyUpgradeable");
9-
await expect(mockGovProxyUpgradeable.initialize()).to.be.reverted(ethers);
15+
await expect(Mock.initialize()).to.be.revert(ethers);
1016
expect(
1117
await ethers.provider.send("eth_getStorageAt", [
12-
await mockGovProxyUpgradeable.getAddress(),
18+
await Mock.getAddress(),
1319
"0xf0c57e16840df040f15088dc2f81fe391c3923bec73e23a9662efc9c229c6a00",
1420
"latest"]
1521
)
1622
).to.eq("0x000000000000000000000000000000000000000000000000ffffffffffffffff");
1723
});
1824

1925
it("Should prevent implementation contract from reinitialization", async function () {
20-
const mockGovProxyUpgradeable = await ethers.deployContract("MockGovProxyUpgradeable");
21-
await expect(mockGovProxyUpgradeable.reinitialize()).to.be.reverted(ethers);
26+
await expect(Mock.reinitialize()).to.be.revert(ethers);
2227
expect(
2328
await ethers.provider.send("eth_getStorageAt", [
24-
await mockGovProxyUpgradeable.getAddress(),
29+
await Mock.getAddress(),
2530
"0xf0c57e16840df040f15088dc2f81fe391c3923bec73e23a9662efc9c229c6a00",
2631
"latest"]
2732
)
2833
).to.eq("0x000000000000000000000000000000000000000000000000ffffffffffffffff");
2934
});
30-
});
35+
});

0 commit comments

Comments
 (0)