Skip to content

Commit 8d8fdd2

Browse files
salman01zpsalman01zp
committed
Update tests
1 parent ef3c2c7 commit 8d8fdd2

File tree

2 files changed

+31
-132
lines changed

2 files changed

+31
-132
lines changed

‎packages/contracts/contracts/utils/Governable.sol

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,10 @@ contract Governable {
6161
_;
6262
}
6363

64-
/// @notice Checks if the vote nonces are valid.
64+
/// @notice Checks if the vote JobId are valid.
6565
modifier areValidVotes(Vote[] memory votes) {
6666
for (uint i = 0; i < votes.length; i++) {
67-
require(votes[i].jobId < jobId, "Governable: JobId of vote must match jobId");
67+
require(votes[i].jobId > jobId, "Governable: JobId of vote must be greater than current jobId");
6868
require(
6969
votes[i].proposedGovernor != address(0x0),
7070
"Governable: Proposed governor cannot be the zero address"
@@ -180,7 +180,7 @@ contract Governable {
180180
// Note: `voterCount` is also assumed to be the maximum # of voters in the system.
181181
// Therefore, if `voterCount / 2` votes are in favor of a new governor, we can
182182
// safely assume that there is no other governor that has more votes.
183-
if (numOfVotesForGovernor[vote.jobId][vote.proposedGovernor] > votingThreshold) {
183+
if (numOfVotesForGovernor[vote.jobId][vote.proposedGovernor] >= votingThreshold) {
184184
_transferOwnership(vote.proposedGovernor, vote.jobId);
185185
// If we transferred ownership, we return true to indicate the election is over.
186186
return true;
@@ -191,19 +191,19 @@ contract Governable {
191191

192192
/// @notice Transfers ownership of the contract to a new account (`newOwner`).
193193
/// @param newOwner The new owner of the contract
194-
/// @param _jobId JobId of the new governor.
195-
function _transferOwnership(address newOwner, uint32 _jobId) internal {
194+
/// @param newJobId JobId of the new governor.
195+
function _transferOwnership(address newOwner, uint32 newJobId) internal {
196196
require(newOwner != address(0), "Governable: New governor is the zero address");
197197
address previousGovernor = governor;
198198
uint32 previousGovernorJobId = jobId;
199199
governor = newOwner;
200200
lastGovernorUpdateTime = block.timestamp;
201-
jobId = _jobId;
201+
jobId = newJobId;
202202
emit GovernanceOwnershipTransferred(
203203
previousGovernor,
204204
previousGovernorJobId,
205205
newOwner,
206-
_jobId,
206+
newJobId,
207207
lastGovernorUpdateTime
208208
);
209209
}

‎packages/contracts/test/governance/governable.test.ts

Lines changed: 24 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ describe('Governable Contract', () => {
3030
arbSigner = signers[2];
3131
// create poseidon hasher
3232
const govFactory = new Governable__factory(wallet);
33-
governableInstance = await govFactory.deploy(sender.address, 0, 0);
33+
governableInstance = await govFactory.deploy(sender.address, 0, 2);
3434
await governableInstance.deployed();
3535
});
3636

@@ -81,26 +81,8 @@ describe('Governable Contract', () => {
8181
'0x' + ethers.utils.keccak256('0x' + dummyPubkey).slice(-40)
8282
);
8383

84-
const refreshProposal = {
85-
voterMerkleRoot: ZERO_BYTES32,
86-
averageSessionLengthInMillisecs: '50000',
87-
voterCount: '1',
88-
nonce: '2',
89-
publicKey: `0x${dummyPubkey}`,
90-
};
91-
92-
const prehashed = solidityPack(
93-
['bytes32', 'uint64', 'uint32', 'uint32', 'bytes'],
94-
[
95-
refreshProposal.voterMerkleRoot,
96-
refreshProposal.averageSessionLengthInMillisecs,
97-
refreshProposal.voterCount,
98-
refreshProposal.nonce,
99-
refreshProposal.publicKey,
100-
]
101-
);
102-
103-
let msg = ethers.utils.arrayify(ethers.utils.keccak256(prehashed));
84+
let newGovernorPublickKey = `0x${dummyPubkey}`;
85+
let msg = ethers.utils.arrayify(ethers.utils.keccak256(newGovernorPublickKey));
10486
let signature = key.sign(msg);
10587
let expandedSig = {
10688
r: '0x' + signature.r.toString('hex'),
@@ -120,43 +102,23 @@ describe('Governable Contract', () => {
120102
}
121103

122104
const tx = await governableInstance.transferOwnershipWithSignature(
123-
refreshProposal.voterMerkleRoot,
124-
refreshProposal.averageSessionLengthInMillisecs,
125-
refreshProposal.voterCount,
126-
refreshProposal.nonce,
127-
refreshProposal.publicKey,
105+
2,
106+
newGovernorPublickKey,
128107
sig
129108
);
130109
await tx.wait();
131110

132111
assert.strictEqual(await governableInstance.governor(), nextGovernorAddress);
133-
assert.strictEqual(
134-
(await governableInstance.voterCount()).toString(),
135-
refreshProposal.voterCount
136-
);
137-
assert.strictEqual(await governableInstance.voterMerkleRoot(), refreshProposal.voterMerkleRoot);
138-
assert.strictEqual((await governableInstance.refreshNonce()).toString(), '2');
139-
112+
assert.strictEqual((await governableInstance.jobId()).toString(), '2');
140113
const filter = governableInstance.filters.GovernanceOwnershipTransferred();
141114
const events = await governableInstance.queryFilter(filter);
142115
assert.strictEqual(nextGovernorAddress, events[2].args.newOwner);
143116
assert.strictEqual(firstRotationKey, events[2].args.previousOwner);
144117
});
145118

146119
it('should vote validly and change the governor', async () => {
147-
const voteStruct = {
148-
nonce: await governableInstance.refreshNonce(),
149-
leafIndex: 0,
150-
siblingPathNodes: ['0x0000000000000000000000000000000000000000000000000000000000000001'],
151-
proposedGovernor: '0x1111111111111111111111111111111111111111',
152-
};
153-
154-
await TruffleAssert.reverts(
155-
governableInstance.voteInFavorForceSetGovernor(voteStruct),
156-
'Invalid time for vote'
157-
);
158-
159-
assert.strictEqual((await governableInstance.refreshNonce()).toString(), '0');
120+
121+
assert.strictEqual((await governableInstance.jobId()).toString(), '0');
160122
const wallet = ethers.Wallet.createRandom();
161123
const key = ec.keyFromPrivate(wallet.privateKey.slice(2), 'hex');
162124
const pubkey = key.getPublic().encode('hex').slice(2);
@@ -174,55 +136,13 @@ describe('Governable Contract', () => {
174136
.encode('hex')
175137
.slice(2);
176138

177-
const nonce = 2;
178-
139+
const jobId = 2; // Job Id of the new governor.
140+
let newGovernorPublickKey = `0x${dummyPubkey}`;
179141
const signers = await ethers.getSigners();
180-
181-
const voter0Signer = signers[0];
182-
const voter0 = signers[0].address;
183-
184142
const voter1Signer = signers[1];
185-
const voter1 = signers[1].address;
186-
187143
const voter2Signer = signers[2];
188-
const voter2 = signers[2].address;
189-
190-
const voter3 = signers[3].address;
191-
192-
const hashVoter0 = ethers.utils.keccak256(voter0);
193-
const hashVoter1 = ethers.utils.keccak256(voter1);
194-
const hashVoter2 = ethers.utils.keccak256(voter2);
195-
const hashVoter3 = ethers.utils.keccak256(voter3);
196-
197-
const hashVoter01 = ethers.utils.keccak256(hashVoter0 + hashVoter1.slice(2));
198-
const hashVoter23 = ethers.utils.keccak256(hashVoter2 + hashVoter3.slice(2));
199-
200-
const hashVoter0123 = ethers.utils.keccak256(hashVoter01 + hashVoter23.slice(2));
201-
202-
const voterMerkleRoot = hashVoter0123;
203-
const averageSessionLengthInMillisecs = 50000;
204-
const voterCount = 4;
205144

206-
const refreshProposal = {
207-
voterMerkleRoot,
208-
averageSessionLengthInMillisecs,
209-
voterCount,
210-
nonce,
211-
publicKey: `0x${dummyPubkey}`,
212-
};
213-
214-
const prehashed = solidityPack(
215-
['bytes32', 'uint64', 'uint32', 'uint32', 'bytes'],
216-
[
217-
refreshProposal.voterMerkleRoot,
218-
refreshProposal.averageSessionLengthInMillisecs,
219-
refreshProposal.voterCount,
220-
refreshProposal.nonce,
221-
refreshProposal.publicKey,
222-
]
223-
);
224-
225-
let msg = ethers.utils.arrayify(ethers.utils.keccak256(prehashed));
145+
let msg = ethers.utils.arrayify(ethers.utils.keccak256(newGovernorPublickKey));
226146
let signature = key.sign(msg);
227147
let expandedSig = {
228148
r: '0x' + signature.r.toString('hex'),
@@ -242,42 +162,18 @@ describe('Governable Contract', () => {
242162
}
243163

244164
const tx = await governableInstance.transferOwnershipWithSignature(
245-
refreshProposal.voterMerkleRoot,
246-
refreshProposal.averageSessionLengthInMillisecs,
247-
refreshProposal.voterCount,
248-
refreshProposal.nonce,
249-
refreshProposal.publicKey,
165+
jobId,
166+
newGovernorPublickKey,
250167
sig
251168
);
252169
await tx.wait();
253170

254-
assert.strictEqual(
255-
(await governableInstance.averageSessionLengthInMillisecs()).toString(),
256-
averageSessionLengthInMillisecs.toString()
257-
);
258-
assert.strictEqual((await governableInstance.voterCount()).toString(), '4');
259-
assert.strictEqual(await governableInstance.voterMerkleRoot(), voterMerkleRoot);
260-
assert.strictEqual((await governableInstance.refreshNonce()).toString(), '2');
261-
await network.provider.send('evm_increaseTime', [600]);
262-
263-
const vote0 = {
264-
nonce: await governableInstance.refreshNonce(),
265-
leafIndex: 0,
266-
siblingPathNodes: [hashVoter1, hashVoter23],
267-
proposedGovernor: '0x1111111111111111111111111111111111111111',
268-
};
269-
270-
await governableInstance.connect(voter0Signer).voteInFavorForceSetGovernor(vote0);
271171

272-
assert.notEqual(
273-
await governableInstance.governor(),
274-
'0x1111111111111111111111111111111111111111'
275-
);
172+
assert.strictEqual((await governableInstance.jobId()).toString(), '2');
173+
await network.provider.send('evm_increaseTime', [600]);
276174

277175
const vote1 = {
278-
nonce: await governableInstance.refreshNonce(),
279-
leafIndex: 1,
280-
siblingPathNodes: [hashVoter0, hashVoter23],
176+
jobId: 3,
281177
proposedGovernor: '0x1111111111111111111111111111111111111111',
282178
};
283179

@@ -289,10 +185,7 @@ describe('Governable Contract', () => {
289185
);
290186

291187
const vote2 = {
292-
nonce: await governableInstance.refreshNonce(),
293-
294-
leafIndex: 2,
295-
siblingPathNodes: [hashVoter3, hashVoter01],
188+
jobId: 3,
296189
proposedGovernor: '0x1111111111111111111111111111111111111111',
297190
};
298191

@@ -302,6 +195,12 @@ describe('Governable Contract', () => {
302195
await governableInstance.governor(),
303196
'0x1111111111111111111111111111111111111111'
304197
);
305-
assert.strictEqual((await governableInstance.refreshNonce()).toString(), '3');
198+
199+
const filter = governableInstance.filters.GovernanceOwnershipTransferred();
200+
const events = await governableInstance.queryFilter(filter);
201+
assert.strictEqual(3, events[3].args.jobId);
202+
assert.strictEqual(2, events[3].args.previousOwnerJobId);
203+
204+
assert.strictEqual((await governableInstance.jobId()).toString(), '3');
306205
});
307206
});

0 commit comments

Comments
 (0)