From 9b0394beb84bbacb59e81b5aef70954e7999c40c Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Thu, 7 Nov 2024 15:38:29 -0500 Subject: [PATCH 01/15] -- duplicated function --- .../test-sol/unit/governance/voting/Election.t.sol | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/protocol/test-sol/unit/governance/voting/Election.t.sol b/packages/protocol/test-sol/unit/governance/voting/Election.t.sol index 9cff990b322..296a4022736 100644 --- a/packages/protocol/test-sol/unit/governance/voting/Election.t.sol +++ b/packages/protocol/test-sol/unit/governance/voting/Election.t.sol @@ -168,14 +168,6 @@ contract ElectionTest is Utils { deployCodeTo("Registry.sol", abi.encode(false), PROXY_ADMIN_ADDRESS); epochManager.initializeSystem(l1EpochNumber, block.number, _elected); } - - function travelNEpoch(uint256 n) public { - if (isL2()) { - epochManager.setCurrentEpochNumber(epochManager.getCurrentEpochNumber() + n); - } else { - blockTravel((n * ph.epochSize()) + 1); - } - } } contract TransitionToL2After is ElectionTest { From 932802450895f93e519ea25c4ff203fbbec82e13 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Tue, 12 Nov 2024 14:26:39 -0500 Subject: [PATCH 02/15] general L2 tests --- .../unit/governance/network/Governance.t.sol | 168 +++++++++++++++--- 1 file changed, 139 insertions(+), 29 deletions(-) diff --git a/packages/protocol/test-sol/unit/governance/network/Governance.t.sol b/packages/protocol/test-sol/unit/governance/network/Governance.t.sol index ae669e956f2..e2b0d4cbfd5 100644 --- a/packages/protocol/test-sol/unit/governance/network/Governance.t.sol +++ b/packages/protocol/test-sol/unit/governance/network/Governance.t.sol @@ -263,6 +263,13 @@ contract GovernanceTest is Test, TestConstants, Utils { } } +contract TransitionToL2After is GovernanceTest { + function setUp() public { + super.setUp(); + _whenL2(); + } +} + contract GovernanceTest_initialize is GovernanceTest { function test_SetsTheOwner() public { assertEq(governance.owner(), accOwner); @@ -368,6 +375,8 @@ contract GovernanceTest_setApprover is GovernanceTest { } } +contract GovernanceTest_setApprover_L2 is TransitionToL2After, GovernanceTest_setApprover {} + contract GovernanceTest_setMinDeposit is GovernanceTest { uint256 NEW_MINDEPOSIT = 45; event MinDepositSet(uint256 minDeposit); @@ -398,6 +407,8 @@ contract GovernanceTest_setMinDeposit is GovernanceTest { } } +contract GovernanceTest_setMinDeposit_L2 is TransitionToL2After, GovernanceTest_setMinDeposit {} + contract GovernanceTest_setConcurrentProposals is GovernanceTest { uint256 NEW_CONCURRENT_PROPOSALS = 45; event ConcurrentProposalsSet(uint256 concurrentProposals); @@ -434,6 +445,11 @@ contract GovernanceTest_setConcurrentProposals is GovernanceTest { } } +contract GovernanceTest_setConcurrentProposals_L2 is + TransitionToL2After, + GovernanceTest_setConcurrentProposals +{} + contract GovernanceTest_setQueueExpiry is GovernanceTest { event QueueExpirySet(uint256 queueExpiry); @@ -469,6 +485,8 @@ contract GovernanceTest_setQueueExpiry is GovernanceTest { } } +contract GovernanceTest_setQueueExpiry_L2 is TransitionToL2After, GovernanceTest_setQueueExpiry {} + contract GovernanceTest_setDequeueFrequency is GovernanceTest { event DequeueFrequencySet(uint256 dequeueFrequency); @@ -504,6 +522,11 @@ contract GovernanceTest_setDequeueFrequency is GovernanceTest { } } +contract GovernanceTest_setDequeueFrequency_L2 is + TransitionToL2After, + GovernanceTest_setDequeueFrequency +{} + contract GovernanceTest_setReferendumStageDuration is GovernanceTest { event ReferendumStageDurationSet(uint256 value); @@ -539,6 +562,11 @@ contract GovernanceTest_setReferendumStageDuration is GovernanceTest { } } +contract GovernanceTest_setReferendumStageDuration_L2 is + TransitionToL2After, + GovernanceTest_setReferendumStageDuration +{} + contract GovernanceTest_setExecutionStageDuration is GovernanceTest { event ExecutionStageDurationSet(uint256 dequeueFrequency); @@ -574,6 +602,11 @@ contract GovernanceTest_setExecutionStageDuration is GovernanceTest { } } +contract GovernanceTest_setExecutionStageDuration_L2 is + TransitionToL2After, + GovernanceTest_setExecutionStageDuration +{} + contract GovernanceTest_setParticipationFloor is GovernanceTest { event ParticipationFloorSet(uint256 value); @@ -604,6 +637,11 @@ contract GovernanceTest_setParticipationFloor is GovernanceTest { } } +contract GovernanceTest_setParticipationFloor_L2 is + TransitionToL2After, + GovernanceTest_setParticipationFloor +{} + contract GovernanceTest_setBaselineUpdateFactor is GovernanceTest { event ParticipationBaselineUpdateFactorSet(uint256 value); @@ -634,6 +672,11 @@ contract GovernanceTest_setBaselineUpdateFactor is GovernanceTest { } } +contract GovernanceTest_setBaselineUpdateFactor_L2 is + TransitionToL2After, + GovernanceTest_setBaselineUpdateFactor +{} + contract GovernanceTest_setBaselineQuorumFactor is GovernanceTest { event ParticipationBaselineQuorumFactorSet(uint256 value); @@ -664,6 +707,11 @@ contract GovernanceTest_setBaselineQuorumFactor is GovernanceTest { } } +contract GovernanceTest_setBaselineQuorumFactor_L2 is + TransitionToL2After, + GovernanceTest_setBaselineQuorumFactor +{} + contract GovernanceTest_setConstitution is GovernanceTest { event ConstitutionSet(address indexed destination, bytes4 indexed functionId, uint256 threshold); @@ -740,6 +788,8 @@ contract GovernanceTest_setConstitution is GovernanceTest { } } +contract GovernanceTest_setConstitution_L2 is TransitionToL2After, GovernanceTest_setConstitution {} + contract GovernanceTest_setSecurityCouncil is GovernanceTest { event SecurityCouncilSet(address indexed council); @@ -785,6 +835,11 @@ contract GovernanceTest_setSecurityCouncil is GovernanceTest { } } +contract GovernanceTest_setSecurityCouncil_L2 is + TransitionToL2After, + GovernanceTest_setSecurityCouncil +{} + contract GovernanceTest_setHotfixExecutionTimeWindow is GovernanceTest { event HotfixExecutionTimeWindowSet(uint256 timeDelta); @@ -815,6 +870,11 @@ contract GovernanceTest_setHotfixExecutionTimeWindow is GovernanceTest { } } +contract GovernanceTest_setHotfixExecutionTimeWindow_L2 is + TransitionToL2After, + GovernanceTest_setHotfixExecutionTimeWindow +{} + contract GovernanceTest_propose is GovernanceTest { event ProposalQueued( uint256 indexed proposalId, @@ -989,6 +1049,8 @@ contract GovernanceTest_propose is GovernanceTest { } } +contract GovernanceTest_propose_L2 is TransitionToL2After, GovernanceTest_propose {} + contract GovernanceTest_upvote is GovernanceTest { event ProposalUpvoted(uint256 indexed proposalId, address indexed account, uint256 upvotes); event ProposalExpired(uint256 indexed proposalId); @@ -1188,6 +1250,8 @@ contract GovernanceTest_upvote is GovernanceTest { } } +contract GovernanceTest_upvote_L2 is TransitionToL2After, GovernanceTest_upvote {} + contract GovernanceTest_revokeUpvote is GovernanceTest { event ProposalExpired(uint256 indexed proposalId); event ProposalUpvoteRevoked( @@ -1277,6 +1341,8 @@ contract GovernanceTest_revokeUpvote is GovernanceTest { } } +contract GovernanceTest_revokeUpvote_L2 is TransitionToL2After, GovernanceTest_revokeUpvote {} + contract GovernanceTest_withdraw is GovernanceTest { address accProposer; @@ -1314,6 +1380,8 @@ contract GovernanceTest_withdraw is GovernanceTest { } } +contract GovernanceTest_withdraw_L2 is TransitionToL2After, GovernanceTest_withdraw {} + contract GovernanceTest_approve is GovernanceTest { uint256 INDEX = 0; // first proposal index @@ -1488,6 +1556,8 @@ contract GovernanceTest_approve is GovernanceTest { } } +contract GovernanceTest_approve_L2 is TransitionToL2After, GovernanceTest_approve {} + contract GovernanceTest_revokeVotes is GovernanceTest { uint256 numVoted; @@ -1616,6 +1686,8 @@ contract GovernanceTest_revokeVotes is GovernanceTest { } } +contract GovernanceTest_revokeVotes_L2 is TransitionToL2After, GovernanceTest_revokeVotes {} + contract GovernanceTest_vote_WhenProposalIsApproved is GovernanceTest { event ProposalVotedV2( uint256 indexed proposalId, @@ -1854,6 +1926,11 @@ contract GovernanceTest_vote_WhenProposalIsApproved is GovernanceTest { } } +contract GovernanceTest_vote_WhenProposalIsApproved_L2 is + TransitionToL2After, + GovernanceTest_vote_WhenProposalIsApproved +{} + contract GovernanceTest_vote_WhenProposalIsApprovedAndHaveSigner is GovernanceTest { address accSigner; @@ -1933,6 +2010,11 @@ contract GovernanceTest_vote_WhenProposalIsApprovedAndHaveSigner is GovernanceTe } } +contract GovernanceTest_vote_WhenProposalIsApprovedAndHaveSigner_L2 is + TransitionToL2After, + GovernanceTest_vote_WhenProposalIsApprovedAndHaveSigner +{} + contract GovernanceTest_vote_WhenProposalIsNotApproved is GovernanceTest { event ProposalVotedV2( uint256 indexed proposalId, @@ -2013,6 +2095,11 @@ contract GovernanceTest_vote_WhenProposalIsNotApproved is GovernanceTest { } } +contract GovernanceTest_vote_WhenProposalIsNotApproved_L2 is + TransitionToL2After, + GovernanceTest_vote_WhenProposalIsNotApproved +{} + contract GovernanceTest_vote_WhenVotingOnDifferentProposalWithSameIndex is GovernanceTest { function test_IgnoreVotesFromPreviousProposal() public { uint256 proposalId1 = makeValidProposal(); @@ -2063,6 +2150,11 @@ contract GovernanceTest_vote_WhenVotingOnDifferentProposalWithSameIndex is Gover } } +contract GovernanceTest_vote_WhenVotingOnDifferentProposalWithSameIndex_L2 is + TransitionToL2After, + GovernanceTest_vote_WhenVotingOnDifferentProposalWithSameIndex +{} + contract GovernanceTest_vote_PartiallyWhenProposalIsApproved is GovernanceTest { event ProposalVotedV2( uint256 indexed proposalId, @@ -2291,6 +2383,11 @@ contract GovernanceTest_vote_PartiallyWhenProposalIsApproved is GovernanceTest { } } +contract GovernanceTest_vote_PartiallyWhenProposalIsApproved_L2 is + TransitionToL2After, + GovernanceTest_vote_PartiallyWhenProposalIsApproved +{} + contract GovernanceTest_votePartially_WhenProposalIsApprovedAndHaveSigner is GovernanceTest { address accSigner; @@ -2405,6 +2502,11 @@ contract GovernanceTest_votePartially_WhenProposalIsApprovedAndHaveSigner is Gov } } +contract GovernanceTest_votePartially_WhenProposalIsApprovedAndHaveSigner_L2 is + TransitionToL2After, + GovernanceTest_votePartially_WhenProposalIsApprovedAndHaveSigner +{} + contract GovernanceTest_votePartially_WhenProposalIsNotApproved is GovernanceTest { event ProposalVotedV2( uint256 indexed proposalId, @@ -2485,6 +2587,11 @@ contract GovernanceTest_votePartially_WhenProposalIsNotApproved is GovernanceTes } } +contract GovernanceTest_votePartially_WhenProposalIsNotApproved_L2 is + TransitionToL2After, + GovernanceTest_votePartially_WhenProposalIsNotApproved +{} + contract GovernanceTest_votePartially_WhenVotingOnDifferentProposalWithSameIndex is GovernanceTest { function test_IgnoreVotesFromPreviousProposal() public { uint256 proposalId1 = makeValidProposal(); @@ -2535,6 +2642,11 @@ contract GovernanceTest_votePartially_WhenVotingOnDifferentProposalWithSameIndex } } +contract GovernanceTest_votePartially_WhenVotingOnDifferentProposalWithSameIndex_L2 is + TransitionToL2After, + GovernanceTest_votePartially_WhenVotingOnDifferentProposalWithSameIndex +{} + contract GovernanceTest_execute is GovernanceTest { event ParticipationBaselineUpdated(uint256 participationBaseline); event ProposalExecuted(uint256 indexed proposalId); @@ -2966,6 +3078,8 @@ contract GovernanceTest_execute is GovernanceTest { } } +contract GovernanceTest_execute_L2 is TransitionToL2After, GovernanceTest_execute {} + contract GovernanceTest_approveHotfix is GovernanceTest { bytes32 constant HOTFIX_HASH = bytes32(uint256(0x123456789)); event HotfixApproved(bytes32 indexed hash, address approver); @@ -3088,7 +3202,7 @@ contract GovernanceTest_whitelistHotfix is GovernanceTest { governance.whitelistHotfix(HOTFIX_HASH); } - function test_Reverts_WhenCalledOnL2() public { + function test_Reverts_WhenL2() public { address validator = actor("validator1"); governance.addValidator(validator); _whenL2(); @@ -3164,7 +3278,7 @@ contract GovernanceTest_hotfixWhitelistValidatorTally is GovernanceTest { assertEq(governance.hotfixWhitelistValidatorTally(HOTFIX_HASH), 3); } - function test_Reverts_WhenCalledOnL2() public { + function test_Reverts_WhenL2() public { address validator = actor("validator1"); governance.addValidator(validator); _whenL2(); @@ -3207,7 +3321,7 @@ contract GovernanceTest_isHotfixPassing is GovernanceTest { assertTrue(governance.isHotfixPassing(HOTFIX_HASH)); } - function test_Reverts_WhenCalledOnL2() public { + function test_Reverts_WhenL2() public { _whenL2(); vm.expectRevert("This method is no longer supported in L2."); governance.isHotfixPassing(HOTFIX_HASH); @@ -3282,7 +3396,7 @@ contract GovernanceTest_prepareHotfix_L2 is GovernanceTest { governance.setSecurityCouncil(accCouncil); } - function test_markHotfixRecordExecutionTimeLimit_whenHotfixApproved() public { + function test_shouldMarkHotfixRecordExecutionTimeLimit_whenHotfixApproved() public { vm.prank(accOwner); governance.setHotfixExecutionTimeWindow(DAY); @@ -3408,8 +3522,10 @@ contract GovernanceTest_resetHotfix is GovernanceTest { vm.prank(accCouncil); governance.approveHotfix(HOTFIX_HASH); + vm.prank(accApprover); governance.approveHotfix(HOTFIX_HASH); + (bool approved, bool councilApproved, , uint256 _preparedTimeLimit) = governance .getL2HotfixRecord(HOTFIX_HASH); @@ -3612,22 +3728,14 @@ contract GovernanceTest_executeHotfix_L2 is GovernanceTest { } function test_ShouldExecuteHotfix_WhenApprovedByApproverAndSecurityCouncil() public { - vm.prank(accApprover); - governance.approveHotfix(hotfixHash); - vm.prank(accCouncil); - governance.approveHotfix(hotfixHash); - governance.prepareHotfix(hotfixHash); + approveAndPrepareHotfix(); executeHotfixTx(); assertEq(testTransactions.getValue(1), 1); } function test_ShouldMarkHotfixAsExecuted_WhenApprovedByApproverAndSecurityCouncil() public { - vm.prank(accApprover); - governance.approveHotfix(hotfixHash); - vm.prank(accCouncil); - governance.approveHotfix(hotfixHash); - governance.prepareHotfix(hotfixHash); + approveAndPrepareHotfix(); executeHotfixTx(); (, , bool executed, ) = governance.getL2HotfixRecord(hotfixHash); @@ -3635,11 +3743,7 @@ contract GovernanceTest_executeHotfix_L2 is GovernanceTest { } function test_Emits_HotfixExecutedEventWhenApprovedByApproverAndSecurityCouncil() public { - vm.prank(accApprover); - governance.approveHotfix(hotfixHash); - vm.prank(accCouncil); - governance.approveHotfix(hotfixHash); - governance.prepareHotfix(hotfixHash); + approveAndPrepareHotfix(); vm.expectEmit(true, true, true, true); emit HotfixExecuted(hotfixHash); @@ -3647,11 +3751,7 @@ contract GovernanceTest_executeHotfix_L2 is GovernanceTest { } function test_Reverts_WhenExecutingSameHotfixTwice() public { - vm.prank(accApprover); - governance.approveHotfix(hotfixHash); - vm.prank(accCouncil); - governance.approveHotfix(hotfixHash); - governance.prepareHotfix(hotfixHash); + approveAndPrepareHotfix(); executeHotfixTx(); vm.expectRevert("hotfix already executed"); @@ -3679,11 +3779,8 @@ contract GovernanceTest_executeHotfix_L2 is GovernanceTest { executeHotfixTx(); } function test_Reverts_WhenExecutedBeyondTheExecutionTimeLimit() public { - vm.prank(accApprover); - governance.approveHotfix(hotfixHash); - vm.prank(accCouncil); - governance.approveHotfix(hotfixHash); - governance.prepareHotfix(hotfixHash); + approveAndPrepareHotfix(); + timeTravel(2 * DAY); vm.expectRevert("Execution time limit has already been reached."); executeHotfixTx(); @@ -3698,6 +3795,14 @@ contract GovernanceTest_executeHotfix_L2 is GovernanceTest { SALT ); } + + function approveAndPrepareHotfix() private { + vm.prank(accApprover); + governance.approveHotfix(hotfixHash); + vm.prank(accCouncil); + governance.approveHotfix(hotfixHash); + governance.prepareHotfix(hotfixHash); + } } contract GovernanceTest_isVoting is GovernanceTest { @@ -4249,3 +4354,8 @@ contract GovernanceTest_removeVotesWhenRevokingDelegatedVotes is GovernanceTest assertVoteRecord(2, proposalIds[2], 0, 0, 51); } } + +contract GovernanceTest_removeVotesWhenRevokingDelegatedVotes_L2 is + TransitionToL2After, + GovernanceTest_removeVotesWhenRevokingDelegatedVotes +{} From eb37996f623799f50464dbfc08257f57f0533ddc Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Tue, 12 Nov 2024 14:37:39 -0500 Subject: [PATCH 03/15] ++ L1 GovernanceTest_resetHotfix --- .../unit/governance/network/Governance.t.sol | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/packages/protocol/test-sol/unit/governance/network/Governance.t.sol b/packages/protocol/test-sol/unit/governance/network/Governance.t.sol index e2b0d4cbfd5..2465db2660a 100644 --- a/packages/protocol/test-sol/unit/governance/network/Governance.t.sol +++ b/packages/protocol/test-sol/unit/governance/network/Governance.t.sol @@ -3502,6 +3502,59 @@ contract GovernanceTest_resetHotfix is GovernanceTest { bytes32 hotfixHash; event HotfixRecordReset(bytes32 indexed hash); + function setUp() public { + super.setUp(); + + address val1 = actor("validator1"); + governance.addValidator(val1); + vm.prank(val1); + accounts.createAccount(); + + // _whenL2(); + vm.prank(accOwner); + governance.setSecurityCouncil(accCouncil); + + hotfixHash = governance.getHotfixHash( + okProp.values, + okProp.destinations, + okProp.data, + okProp.dataLengths, + SALT + ); + } + + function test_Reverts_whenCalledOnL1() public { + vm.prank(accOwner); + governance.setHotfixExecutionTimeWindow(DAY); + + vm.prank(accApprover); + governance.approveHotfix(HOTFIX_HASH); + + (bool approved, , ) = governance.getHotfixRecord(HOTFIX_HASH); + + assertTrue(approved); + + vm.roll(block.number + governance.getEpochSize()); + vm.prank(actor("validator1")); + governance.whitelistHotfix(HOTFIX_HASH); + + uint256 epoch = governance.getEpochNumber(); + + governance.prepareHotfix(HOTFIX_HASH); + + timeTravel(DAY + 1); + + vm.expectRevert("hotfix not prepared"); + governance.resetHotFixRecord(HOTFIX_HASH); + } +} + +contract GovernanceTest_resetHotfix_L2 is GovernanceTest { + bytes32 constant HOTFIX_HASH = bytes32(uint256(0x123456789)); + bytes32 constant SALT = 0x657ed9d64e84fa3d1af43b3a307db22aba2d90a158015df1c588c02e24ca08f0; + bytes32 hotfixHash; + event HotfixRecordReset(bytes32 indexed hash); + function setUp() public { super.setUp(); _whenL2(); From f7d886d4abf386beb8924c6f91dd522affa22ef8 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Tue, 12 Nov 2024 14:42:16 -0500 Subject: [PATCH 04/15] ++ remaining tests --- .../unit/governance/network/Governance.t.sol | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/packages/protocol/test-sol/unit/governance/network/Governance.t.sol b/packages/protocol/test-sol/unit/governance/network/Governance.t.sol index 2465db2660a..68a9eef38ab 100644 --- a/packages/protocol/test-sol/unit/governance/network/Governance.t.sol +++ b/packages/protocol/test-sol/unit/governance/network/Governance.t.sol @@ -3912,6 +3912,8 @@ contract GovernanceTest_isVoting is GovernanceTest { } } +contract GovernanceTest_isVoting_L2 is TransitionToL2After, GovernanceTest_isVoting {} + contract GovernanceTest_isProposalPassing is GovernanceTest { address accSndVoter; @@ -3954,6 +3956,11 @@ contract GovernanceTest_isProposalPassing is GovernanceTest { } } +contract GovernanceTest_isProposalPassing_L2 is + TransitionToL2After, + GovernanceTest_isProposalPassing +{} + contract GovernanceTest_dequeueProposalsIfReady is GovernanceTest { function test_notUpdateLastDequeueWhenThereAreNoQueuedProposals() public { uint256 originalLastDequeue = governance.lastDequeue(); @@ -3987,6 +3994,11 @@ contract GovernanceTest_dequeueProposalsIfReady is GovernanceTest { } } +contract GovernanceTest_dequeueProposalsIfReady_L2 is + TransitionToL2After, + GovernanceTest_dequeueProposalsIfReady +{} + contract GovernanceTest_getProposalStage is GovernanceTest { function test_returnNoneStageWhenProposalDoesNotExists() public { assertEq(uint256(governance.getProposalStage(0)), uint256(Proposals.Stage.None)); @@ -4119,6 +4131,11 @@ contract GovernanceTest_getProposalStage is GovernanceTest { } } +contract GovernanceTest_getProposalStage_L2 is + TransitionToL2After, + GovernanceTest_getProposalStage +{} + contract GovernanceTest_getAmountOfGoldUsedForVoting is GovernanceTest { function test_showCorrectNumberOfVotes_whenVotingOn1ConcurrentProposal() public { makeAndApprove3ConcurrentProposals(); @@ -4247,6 +4264,11 @@ contract GovernanceTest_getAmountOfGoldUsedForVoting is GovernanceTest { } } +contract GovernanceTest_getAmountOfGoldUsedForVoting_L2 is + TransitionToL2After, + GovernanceTest_getAmountOfGoldUsedForVoting +{} + contract GovernanceTest_removeVotesWhenRevokingDelegatedVotes is GovernanceTest { uint256[] proposalIds; From 0ad41744c807151b31096e689bfd17577f1c6ebd Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Tue, 12 Nov 2024 18:00:10 -0500 Subject: [PATCH 05/15] debuging --- packages/celotool/src/e2e-tests/governance_tests.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/celotool/src/e2e-tests/governance_tests.ts b/packages/celotool/src/e2e-tests/governance_tests.ts index 3bdffa5fb79..39ef34b11e7 100644 --- a/packages/celotool/src/e2e-tests/governance_tests.ts +++ b/packages/celotool/src/e2e-tests/governance_tests.ts @@ -310,7 +310,7 @@ describe('governance tests', () => { if (myceloAddress === groupAddress) { return '0x' + generatePrivateKey(mnemonic, AccountType.VALIDATOR_GROUP, 0) } - // Otherwise, the validator group key is encoded in its name (see 25_elect_validators.ts) + // Otherwise, the validator group key is encoded in its name (see 30_elect_validators.ts) const name = await accounts.methods.getName(groupAddress).call() const encryptedKeystore64 = name.split(' ')[1] const encryptedKeystore = JSON.parse(Buffer.from(encryptedKeystore64, 'base64').toString()) @@ -515,7 +515,7 @@ describe('governance tests', () => { assert.equal(validatorSetSize, groupMembership.length) } }) - + // TODO (soloseng) fix test such that it returns expected validators it('should always return a validator set equal to the signing keys of the group members at the end of the last epoch', async function (this: any) { this.timeout(0) for (const blockNumber of blockNumbers) { @@ -538,11 +538,13 @@ describe('governance tests', () => { // Fetch the round robin order if it hasn't already been set for this epoch. if (roundRobinOrder.length === 0 || blockNumber === lastEpochBlock + 1) { const validatorSet = await getValidatorSetSignersAtBlock(blockNumber) + console.log(`### validatorSet: ${validatorSet}`) roundRobinOrder = await Promise.all( validatorSet.map( async (_, i) => (await web3.eth.getBlock(lastEpochBlock + i + 1)).miner ) ) + console.log(`### roundRobinOrder: ${roundRobinOrder}`) assert.sameMembers(roundRobinOrder, validatorSet) } const indexInEpoch = blockNumber - lastEpochBlock - 1 From dd47374ee8b8cbcc72347bc67bd3d7208aefe501 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Wed, 13 Nov 2024 08:41:19 -0500 Subject: [PATCH 06/15] more debug logs --- packages/celotool/src/e2e-tests/governance_tests.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/celotool/src/e2e-tests/governance_tests.ts b/packages/celotool/src/e2e-tests/governance_tests.ts index 39ef34b11e7..f323499e78d 100644 --- a/packages/celotool/src/e2e-tests/governance_tests.ts +++ b/packages/celotool/src/e2e-tests/governance_tests.ts @@ -521,11 +521,18 @@ describe('governance tests', () => { for (const blockNumber of blockNumbers) { const lastEpochBlock = getLastEpochBlock(blockNumber, epoch) const memberAccounts = await getValidatorGroupMembers(lastEpochBlock) + console.log(`### Got memberAccounts: ${memberAccounts}`) const memberSigners = await Promise.all( memberAccounts.map((v: string) => getValidatorSigner(v, lastEpochBlock)) ) + console.log(`### Got memberSigners: ${memberSigners}`) + const validatorSetSigners = await getValidatorSetSignersAtBlock(blockNumber) + console.log(`### Got validatorSetSigners: ${validatorSetSigners}`) + const validatorSetAccounts = await getValidatorSetAccountsAtBlock(blockNumber) + console.log(`### Got validatorSetAccounts: ${validatorSetAccounts}`) + assert.sameMembers(memberSigners, validatorSetSigners) assert.sameMembers(memberAccounts, validatorSetAccounts) } From 20f882d7090e8645128a46add268695919a2bce2 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Wed, 13 Nov 2024 11:03:16 -0500 Subject: [PATCH 07/15] revert debug logs --- packages/celotool/src/e2e-tests/governance_tests.ts | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/packages/celotool/src/e2e-tests/governance_tests.ts b/packages/celotool/src/e2e-tests/governance_tests.ts index f323499e78d..3bdffa5fb79 100644 --- a/packages/celotool/src/e2e-tests/governance_tests.ts +++ b/packages/celotool/src/e2e-tests/governance_tests.ts @@ -310,7 +310,7 @@ describe('governance tests', () => { if (myceloAddress === groupAddress) { return '0x' + generatePrivateKey(mnemonic, AccountType.VALIDATOR_GROUP, 0) } - // Otherwise, the validator group key is encoded in its name (see 30_elect_validators.ts) + // Otherwise, the validator group key is encoded in its name (see 25_elect_validators.ts) const name = await accounts.methods.getName(groupAddress).call() const encryptedKeystore64 = name.split(' ')[1] const encryptedKeystore = JSON.parse(Buffer.from(encryptedKeystore64, 'base64').toString()) @@ -515,24 +515,17 @@ describe('governance tests', () => { assert.equal(validatorSetSize, groupMembership.length) } }) - // TODO (soloseng) fix test such that it returns expected validators + it('should always return a validator set equal to the signing keys of the group members at the end of the last epoch', async function (this: any) { this.timeout(0) for (const blockNumber of blockNumbers) { const lastEpochBlock = getLastEpochBlock(blockNumber, epoch) const memberAccounts = await getValidatorGroupMembers(lastEpochBlock) - console.log(`### Got memberAccounts: ${memberAccounts}`) const memberSigners = await Promise.all( memberAccounts.map((v: string) => getValidatorSigner(v, lastEpochBlock)) ) - console.log(`### Got memberSigners: ${memberSigners}`) - const validatorSetSigners = await getValidatorSetSignersAtBlock(blockNumber) - console.log(`### Got validatorSetSigners: ${validatorSetSigners}`) - const validatorSetAccounts = await getValidatorSetAccountsAtBlock(blockNumber) - console.log(`### Got validatorSetAccounts: ${validatorSetAccounts}`) - assert.sameMembers(memberSigners, validatorSetSigners) assert.sameMembers(memberAccounts, validatorSetAccounts) } @@ -545,13 +538,11 @@ describe('governance tests', () => { // Fetch the round robin order if it hasn't already been set for this epoch. if (roundRobinOrder.length === 0 || blockNumber === lastEpochBlock + 1) { const validatorSet = await getValidatorSetSignersAtBlock(blockNumber) - console.log(`### validatorSet: ${validatorSet}`) roundRobinOrder = await Promise.all( validatorSet.map( async (_, i) => (await web3.eth.getBlock(lastEpochBlock + i + 1)).miner ) ) - console.log(`### roundRobinOrder: ${roundRobinOrder}`) assert.sameMembers(roundRobinOrder, validatorSet) } const indexInEpoch = blockNumber - lastEpochBlock - 1 From d3a69d81045067cb8e38ec8eac40ed8a8c5c8b65 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Wed, 13 Nov 2024 11:12:59 -0500 Subject: [PATCH 08/15] change inheritance order --- .../unit/governance/voting/Election.t.sol | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/packages/protocol/test-sol/unit/governance/voting/Election.t.sol b/packages/protocol/test-sol/unit/governance/voting/Election.t.sol index 296a4022736..b6db001ba09 100644 --- a/packages/protocol/test-sol/unit/governance/voting/Election.t.sol +++ b/packages/protocol/test-sol/unit/governance/voting/Election.t.sol @@ -222,8 +222,8 @@ contract ElectionTest_SetElectabilityThreshold is ElectionTest { } contract ElectionTest_SetElectabilityThreshold_L2 is - ElectionTest_SetElectabilityThreshold, - TransitionToL2After + TransitionToL2After, + ElectionTest_SetElectabilityThreshold {} contract ElectionTest_SetElectableValidators is ElectionTest { @@ -267,8 +267,8 @@ contract ElectionTest_SetElectableValidators is ElectionTest { } contract ElectionTest_SetElectableValidators_L2 is - ElectionTest_SetElectableValidators, - TransitionToL2After + TransitionToL2After, + ElectionTest_SetElectableValidators {} contract ElectionTest_SetMaxNumGroupsVotedFor is ElectionTest { @@ -298,8 +298,8 @@ contract ElectionTest_SetMaxNumGroupsVotedFor is ElectionTest { } contract ElectionTest_SetMaxNumGroupsVotedFor_L2 is - ElectionTest_SetMaxNumGroupsVotedFor, - TransitionToL2After + TransitionToL2After, + ElectionTest_SetMaxNumGroupsVotedFor {} contract ElectionTest_SetAllowedToVoteOverMaxNumberOfGroups is ElectionTest { @@ -342,8 +342,8 @@ contract ElectionTest_SetAllowedToVoteOverMaxNumberOfGroups is ElectionTest { } contract ElectionTest_SetAllowedToVoteOverMaxNumberOfGroups_L2 is - ElectionTest_SetAllowedToVoteOverMaxNumberOfGroups, - TransitionToL2After + TransitionToL2After, + ElectionTest_SetAllowedToVoteOverMaxNumberOfGroups {} contract ElectionTest_MarkGroupEligible is ElectionTest { @@ -382,7 +382,7 @@ contract ElectionTest_MarkGroupEligible is ElectionTest { } } -contract ElectionTest_MarkGroupEligible_L2 is ElectionTest_MarkGroupEligible, TransitionToL2After {} +contract ElectionTest_MarkGroupEligible_L2 is TransitionToL2After, ElectionTest_MarkGroupEligible {} contract ElectionTest_MarkGroupInEligible is ElectionTest { function setUp() public { @@ -421,8 +421,8 @@ contract ElectionTest_MarkGroupInEligible is ElectionTest { } contract ElectionTest_MarkGroupInEligible_L2 is - ElectionTest_MarkGroupInEligible, - TransitionToL2After + TransitionToL2After, + ElectionTest_MarkGroupInEligible {} contract ElectionTest_Vote_WhenGroupEligible is ElectionTest { @@ -784,8 +784,8 @@ contract ElectionTest_Vote_WhenGroupEligible_WhenGroupCanReceiveVotes is Electio } contract ElectionTest_Vote_WhenGroupEligible_WhenGroupCanReceiveVotes_L2 is - ElectionTest_Vote_WhenGroupEligible_WhenGroupCanReceiveVotes, - TransitionToL2After + TransitionToL2After, + ElectionTest_Vote_WhenGroupEligible_WhenGroupCanReceiveVotes {} contract ElectionTest_Vote_GroupNotEligible is ElectionTest { @@ -812,8 +812,8 @@ contract ElectionTest_Vote_GroupNotEligible is ElectionTest { } contract ElectionTest_Vote_GroupNotEligible_L2 is - ElectionTest_Vote_GroupNotEligible, - TransitionToL2After + TransitionToL2After, + ElectionTest_Vote_GroupNotEligible {} contract ElectionTest_Activate is ElectionTest { @@ -1297,7 +1297,7 @@ contract ElectionTest_RevokePending is ElectionTest { } } -contract ElectionTest_RevokePending_L2 is ElectionTest_RevokePending, TransitionToL2After {} +contract ElectionTest_RevokePending_L2 is TransitionToL2After, ElectionTest_RevokePending {} contract ElectionTest_RevokeActive is ElectionTest { address voter0 = address(this); @@ -1840,8 +1840,8 @@ contract ElectionTest_ElectValidatorSigners is ElectionTest_ElectValidatorsAbstr } contract ElectionTest_ElectValidatorSignersL2 is - ElectionTest_ElectValidatorSigners, - TransitionToL2After + TransitionToL2After, + ElectionTest_ElectValidatorSigners {} contract ElectionTest_ElectValidatorsAccounts is ElectionTest_ElectValidatorsAbstract { @@ -1956,8 +1956,8 @@ contract ElectionTest_ElectValidatorsAccounts is ElectionTest_ElectValidatorsAbs } contract ElectionTest_ElectValidatorsAccountsL2 is - ElectionTest_ElectValidatorsAccounts, - TransitionToL2After + TransitionToL2After, + ElectionTest_ElectValidatorsAccounts {} contract ElectionTest_GetGroupEpochRewards is ElectionTest { From ea6b08148f10f3ac7918fe869070b69902339a8d Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Wed, 13 Nov 2024 11:35:30 -0500 Subject: [PATCH 09/15] renaming of contract --- .../unit/governance/network/Governance.t.sol | 73 ++++++++++--------- 1 file changed, 38 insertions(+), 35 deletions(-) diff --git a/packages/protocol/test-sol/unit/governance/network/Governance.t.sol b/packages/protocol/test-sol/unit/governance/network/Governance.t.sol index 68a9eef38ab..5dc6a083774 100644 --- a/packages/protocol/test-sol/unit/governance/network/Governance.t.sol +++ b/packages/protocol/test-sol/unit/governance/network/Governance.t.sol @@ -263,7 +263,7 @@ contract GovernanceTest is Test, TestConstants, Utils { } } -contract TransitionToL2After is GovernanceTest { +contract TransitionToL2AfterL1 is GovernanceTest { function setUp() public { super.setUp(); _whenL2(); @@ -375,7 +375,7 @@ contract GovernanceTest_setApprover is GovernanceTest { } } -contract GovernanceTest_setApprover_L2 is TransitionToL2After, GovernanceTest_setApprover {} +contract GovernanceTest_setApprover_L2 is TransitionToL2AfterL1, GovernanceTest_setApprover {} contract GovernanceTest_setMinDeposit is GovernanceTest { uint256 NEW_MINDEPOSIT = 45; @@ -407,7 +407,7 @@ contract GovernanceTest_setMinDeposit is GovernanceTest { } } -contract GovernanceTest_setMinDeposit_L2 is TransitionToL2After, GovernanceTest_setMinDeposit {} +contract GovernanceTest_setMinDeposit_L2 is TransitionToL2AfterL1, GovernanceTest_setMinDeposit {} contract GovernanceTest_setConcurrentProposals is GovernanceTest { uint256 NEW_CONCURRENT_PROPOSALS = 45; @@ -446,7 +446,7 @@ contract GovernanceTest_setConcurrentProposals is GovernanceTest { } contract GovernanceTest_setConcurrentProposals_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_setConcurrentProposals {} @@ -485,7 +485,7 @@ contract GovernanceTest_setQueueExpiry is GovernanceTest { } } -contract GovernanceTest_setQueueExpiry_L2 is TransitionToL2After, GovernanceTest_setQueueExpiry {} +contract GovernanceTest_setQueueExpiry_L2 is TransitionToL2AfterL1, GovernanceTest_setQueueExpiry {} contract GovernanceTest_setDequeueFrequency is GovernanceTest { event DequeueFrequencySet(uint256 dequeueFrequency); @@ -523,7 +523,7 @@ contract GovernanceTest_setDequeueFrequency is GovernanceTest { } contract GovernanceTest_setDequeueFrequency_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_setDequeueFrequency {} @@ -563,7 +563,7 @@ contract GovernanceTest_setReferendumStageDuration is GovernanceTest { } contract GovernanceTest_setReferendumStageDuration_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_setReferendumStageDuration {} @@ -603,7 +603,7 @@ contract GovernanceTest_setExecutionStageDuration is GovernanceTest { } contract GovernanceTest_setExecutionStageDuration_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_setExecutionStageDuration {} @@ -638,7 +638,7 @@ contract GovernanceTest_setParticipationFloor is GovernanceTest { } contract GovernanceTest_setParticipationFloor_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_setParticipationFloor {} @@ -673,7 +673,7 @@ contract GovernanceTest_setBaselineUpdateFactor is GovernanceTest { } contract GovernanceTest_setBaselineUpdateFactor_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_setBaselineUpdateFactor {} @@ -708,7 +708,7 @@ contract GovernanceTest_setBaselineQuorumFactor is GovernanceTest { } contract GovernanceTest_setBaselineQuorumFactor_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_setBaselineQuorumFactor {} @@ -788,7 +788,10 @@ contract GovernanceTest_setConstitution is GovernanceTest { } } -contract GovernanceTest_setConstitution_L2 is TransitionToL2After, GovernanceTest_setConstitution {} +contract GovernanceTest_setConstitution_L2 is + TransitionToL2AfterL1, + GovernanceTest_setConstitution +{} contract GovernanceTest_setSecurityCouncil is GovernanceTest { event SecurityCouncilSet(address indexed council); @@ -836,7 +839,7 @@ contract GovernanceTest_setSecurityCouncil is GovernanceTest { } contract GovernanceTest_setSecurityCouncil_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_setSecurityCouncil {} @@ -871,7 +874,7 @@ contract GovernanceTest_setHotfixExecutionTimeWindow is GovernanceTest { } contract GovernanceTest_setHotfixExecutionTimeWindow_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_setHotfixExecutionTimeWindow {} @@ -1049,7 +1052,7 @@ contract GovernanceTest_propose is GovernanceTest { } } -contract GovernanceTest_propose_L2 is TransitionToL2After, GovernanceTest_propose {} +contract GovernanceTest_propose_L2 is TransitionToL2AfterL1, GovernanceTest_propose {} contract GovernanceTest_upvote is GovernanceTest { event ProposalUpvoted(uint256 indexed proposalId, address indexed account, uint256 upvotes); @@ -1250,7 +1253,7 @@ contract GovernanceTest_upvote is GovernanceTest { } } -contract GovernanceTest_upvote_L2 is TransitionToL2After, GovernanceTest_upvote {} +contract GovernanceTest_upvote_L2 is TransitionToL2AfterL1, GovernanceTest_upvote {} contract GovernanceTest_revokeUpvote is GovernanceTest { event ProposalExpired(uint256 indexed proposalId); @@ -1341,7 +1344,7 @@ contract GovernanceTest_revokeUpvote is GovernanceTest { } } -contract GovernanceTest_revokeUpvote_L2 is TransitionToL2After, GovernanceTest_revokeUpvote {} +contract GovernanceTest_revokeUpvote_L2 is TransitionToL2AfterL1, GovernanceTest_revokeUpvote {} contract GovernanceTest_withdraw is GovernanceTest { address accProposer; @@ -1380,7 +1383,7 @@ contract GovernanceTest_withdraw is GovernanceTest { } } -contract GovernanceTest_withdraw_L2 is TransitionToL2After, GovernanceTest_withdraw {} +contract GovernanceTest_withdraw_L2 is TransitionToL2AfterL1, GovernanceTest_withdraw {} contract GovernanceTest_approve is GovernanceTest { uint256 INDEX = 0; // first proposal index @@ -1556,7 +1559,7 @@ contract GovernanceTest_approve is GovernanceTest { } } -contract GovernanceTest_approve_L2 is TransitionToL2After, GovernanceTest_approve {} +contract GovernanceTest_approve_L2 is TransitionToL2AfterL1, GovernanceTest_approve {} contract GovernanceTest_revokeVotes is GovernanceTest { uint256 numVoted; @@ -1686,7 +1689,7 @@ contract GovernanceTest_revokeVotes is GovernanceTest { } } -contract GovernanceTest_revokeVotes_L2 is TransitionToL2After, GovernanceTest_revokeVotes {} +contract GovernanceTest_revokeVotes_L2 is TransitionToL2AfterL1, GovernanceTest_revokeVotes {} contract GovernanceTest_vote_WhenProposalIsApproved is GovernanceTest { event ProposalVotedV2( @@ -1927,7 +1930,7 @@ contract GovernanceTest_vote_WhenProposalIsApproved is GovernanceTest { } contract GovernanceTest_vote_WhenProposalIsApproved_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_vote_WhenProposalIsApproved {} @@ -2011,7 +2014,7 @@ contract GovernanceTest_vote_WhenProposalIsApprovedAndHaveSigner is GovernanceTe } contract GovernanceTest_vote_WhenProposalIsApprovedAndHaveSigner_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_vote_WhenProposalIsApprovedAndHaveSigner {} @@ -2096,7 +2099,7 @@ contract GovernanceTest_vote_WhenProposalIsNotApproved is GovernanceTest { } contract GovernanceTest_vote_WhenProposalIsNotApproved_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_vote_WhenProposalIsNotApproved {} @@ -2151,7 +2154,7 @@ contract GovernanceTest_vote_WhenVotingOnDifferentProposalWithSameIndex is Gover } contract GovernanceTest_vote_WhenVotingOnDifferentProposalWithSameIndex_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_vote_WhenVotingOnDifferentProposalWithSameIndex {} @@ -2384,7 +2387,7 @@ contract GovernanceTest_vote_PartiallyWhenProposalIsApproved is GovernanceTest { } contract GovernanceTest_vote_PartiallyWhenProposalIsApproved_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_vote_PartiallyWhenProposalIsApproved {} @@ -2503,7 +2506,7 @@ contract GovernanceTest_votePartially_WhenProposalIsApprovedAndHaveSigner is Gov } contract GovernanceTest_votePartially_WhenProposalIsApprovedAndHaveSigner_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_votePartially_WhenProposalIsApprovedAndHaveSigner {} @@ -2588,7 +2591,7 @@ contract GovernanceTest_votePartially_WhenProposalIsNotApproved is GovernanceTes } contract GovernanceTest_votePartially_WhenProposalIsNotApproved_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_votePartially_WhenProposalIsNotApproved {} @@ -2643,7 +2646,7 @@ contract GovernanceTest_votePartially_WhenVotingOnDifferentProposalWithSameIndex } contract GovernanceTest_votePartially_WhenVotingOnDifferentProposalWithSameIndex_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_votePartially_WhenVotingOnDifferentProposalWithSameIndex {} @@ -3078,7 +3081,7 @@ contract GovernanceTest_execute is GovernanceTest { } } -contract GovernanceTest_execute_L2 is TransitionToL2After, GovernanceTest_execute {} +contract GovernanceTest_execute_L2 is TransitionToL2AfterL1, GovernanceTest_execute {} contract GovernanceTest_approveHotfix is GovernanceTest { bytes32 constant HOTFIX_HASH = bytes32(uint256(0x123456789)); @@ -3912,7 +3915,7 @@ contract GovernanceTest_isVoting is GovernanceTest { } } -contract GovernanceTest_isVoting_L2 is TransitionToL2After, GovernanceTest_isVoting {} +contract GovernanceTest_isVoting_L2 is TransitionToL2AfterL1, GovernanceTest_isVoting {} contract GovernanceTest_isProposalPassing is GovernanceTest { address accSndVoter; @@ -3957,7 +3960,7 @@ contract GovernanceTest_isProposalPassing is GovernanceTest { } contract GovernanceTest_isProposalPassing_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_isProposalPassing {} @@ -3995,7 +3998,7 @@ contract GovernanceTest_dequeueProposalsIfReady is GovernanceTest { } contract GovernanceTest_dequeueProposalsIfReady_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_dequeueProposalsIfReady {} @@ -4132,7 +4135,7 @@ contract GovernanceTest_getProposalStage is GovernanceTest { } contract GovernanceTest_getProposalStage_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_getProposalStage {} @@ -4265,7 +4268,7 @@ contract GovernanceTest_getAmountOfGoldUsedForVoting is GovernanceTest { } contract GovernanceTest_getAmountOfGoldUsedForVoting_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_getAmountOfGoldUsedForVoting {} @@ -4431,6 +4434,6 @@ contract GovernanceTest_removeVotesWhenRevokingDelegatedVotes is GovernanceTest } contract GovernanceTest_removeVotesWhenRevokingDelegatedVotes_L2 is - TransitionToL2After, + TransitionToL2AfterL1, GovernanceTest_removeVotesWhenRevokingDelegatedVotes {} From d6e87c53d6fd8e4265397775a852e7127057ff6c Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Thu, 14 Nov 2024 12:54:58 -0500 Subject: [PATCH 10/15] PR feedback --- .../unit/governance/network/Governance.t.sol | 45 ++++++++++--------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/packages/protocol/test-sol/unit/governance/network/Governance.t.sol b/packages/protocol/test-sol/unit/governance/network/Governance.t.sol index 5dc6a083774..168956da476 100644 --- a/packages/protocol/test-sol/unit/governance/network/Governance.t.sol +++ b/packages/protocol/test-sol/unit/governance/network/Governance.t.sol @@ -3292,17 +3292,19 @@ contract GovernanceTest_hotfixWhitelistValidatorTally is GovernanceTest { contract GovernanceTest_isHotfixPassing is GovernanceTest { bytes32 constant HOTFIX_HASH = bytes32(uint256(0x123456789)); + address validator1; + address validator2; function setUp() public { super.setUp(); - address val1 = actor("validator1"); - governance.addValidator(val1); - vm.prank(val1); + validator1 = actor("validator1"); + governance.addValidator(validator1); + vm.prank(validator1); accounts.createAccount(); - address val2 = actor("validator2"); - governance.addValidator(val2); - vm.prank(val2); + validator2 = actor("validator2"); + governance.addValidator(validator2); + vm.prank(validator2); accounts.createAccount(); } @@ -3311,15 +3313,15 @@ contract GovernanceTest_isHotfixPassing is GovernanceTest { } function test_returnFalseWhenHotfixHasBeenWhitelistedButNotByQuorum() public { - vm.prank(actor("validator1")); + vm.prank(validator1); governance.whitelistHotfix(HOTFIX_HASH); assertFalse(governance.isHotfixPassing(HOTFIX_HASH)); } function test_returnTrueWhenHotfixIsWhitelistedByQuorum() public { - vm.prank(actor("validator1")); + vm.prank(validator1); governance.whitelistHotfix(HOTFIX_HASH); - vm.prank(actor("validator2")); + vm.prank(validator2); governance.whitelistHotfix(HOTFIX_HASH); assertTrue(governance.isHotfixPassing(HOTFIX_HASH)); } @@ -3333,19 +3335,20 @@ contract GovernanceTest_isHotfixPassing is GovernanceTest { contract GovernanceTest_prepareHotfix is GovernanceTest { bytes32 constant HOTFIX_HASH = bytes32(uint256(0x123456789)); + address validator1; event HotfixPrepared(bytes32 indexed hash, uint256 indexed epoch); function setUp() public { super.setUp(); - address val1 = actor("validator1"); - governance.addValidator(val1); - vm.prank(val1); + validator1 = actor("validator1"); + governance.addValidator(validator1); + vm.prank(validator1); accounts.createAccount(); } function test_markHotfixRecordPreparedEpoch_whenHotfixIsPassing() public { vm.roll(block.number + governance.getEpochSize()); - vm.prank(actor("validator1")); + vm.prank(validator1); governance.whitelistHotfix(HOTFIX_HASH); governance.prepareHotfix(HOTFIX_HASH); (, , uint256 preparedEpoch) = governance.getL1HotfixRecord(HOTFIX_HASH); @@ -3355,7 +3358,7 @@ contract GovernanceTest_prepareHotfix is GovernanceTest { function test_emitHotfixPreparedEvent_whenHotfixIsPassing() public { vm.roll(block.number + governance.getEpochSize()); - vm.prank(actor("validator1")); + vm.prank(validator1); governance.whitelistHotfix(HOTFIX_HASH); uint256 epoch = governance.getEpochNumber(); @@ -3366,7 +3369,7 @@ contract GovernanceTest_prepareHotfix is GovernanceTest { function test_succeedForEpochDifferentPreparedEpoch_whenHotfixIsPassing() public { vm.roll(block.number + governance.getEpochSize()); - vm.prank(actor("validator1")); + vm.prank(validator1); governance.whitelistHotfix(HOTFIX_HASH); governance.prepareHotfix(HOTFIX_HASH); vm.roll(block.number + governance.getEpochSize()); @@ -3380,7 +3383,7 @@ contract GovernanceTest_prepareHotfix is GovernanceTest { function test_Reverts_IfEpochEqualsPreparedEpoch_whenHotfixIsPassing() public { vm.roll(block.number + governance.getEpochSize()); - vm.prank(actor("validator1")); + vm.prank(validator1); governance.whitelistHotfix(HOTFIX_HASH); governance.prepareHotfix(HOTFIX_HASH); vm.expectRevert("hotfix already prepared for this epoch"); @@ -3503,17 +3506,17 @@ contract GovernanceTest_resetHotfix is GovernanceTest { bytes32 constant HOTFIX_HASH = bytes32(uint256(0x123456789)); bytes32 constant SALT = 0x657ed9d64e84fa3d1af43b3a307db22aba2d90a158015df1c588c02e24ca08f0; bytes32 hotfixHash; + address validator1; event HotfixRecordReset(bytes32 indexed hash); function setUp() public { super.setUp(); - address val1 = actor("validator1"); - governance.addValidator(val1); - vm.prank(val1); + validator1 = actor("validator1"); + governance.addValidator(validator1); + vm.prank(validator1); accounts.createAccount(); - // _whenL2(); vm.prank(accOwner); governance.setSecurityCouncil(accCouncil); @@ -3538,7 +3541,7 @@ contract GovernanceTest_resetHotfix is GovernanceTest { assertTrue(approved); vm.roll(block.number + governance.getEpochSize()); - vm.prank(actor("validator1")); + vm.prank(validator1); governance.whitelistHotfix(HOTFIX_HASH); uint256 epoch = governance.getEpochNumber(); From 5b2d69f45a4b10a9952753c665cb54263a2930f6 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Thu, 14 Nov 2024 13:10:38 -0500 Subject: [PATCH 11/15] -- commented code --- .../test-sol/unit/governance/voting/Election.t.sol | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/protocol/test-sol/unit/governance/voting/Election.t.sol b/packages/protocol/test-sol/unit/governance/voting/Election.t.sol index 03b6b9c2164..6e4fc539fbb 100644 --- a/packages/protocol/test-sol/unit/governance/voting/Election.t.sol +++ b/packages/protocol/test-sol/unit/governance/voting/Election.t.sol @@ -157,14 +157,6 @@ contract ElectionTest is Utils { blocker = new TestBlocker(); election.setBlockedByContract(address(blocker)); } - - // function travelNEpoch(uint256 n) public { - // if (isL2()) { - // epochManager.setCurrentEpochNumber(epochManager.getCurrentEpochNumber() + n); - // } else { - // blockTravel((n * ph.epochSize()) + 1); - // } - // } } contract ElectionTest_L2 is ElectionTest, WhenL2 {} From 4191d58069e25d59a5ee1702b6670529dfdeece0 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:27:02 -0500 Subject: [PATCH 12/15] -- unused imports --- .../test-sol/unit/governance/validators/Validators.t.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/protocol/test-sol/unit/governance/validators/Validators.t.sol b/packages/protocol/test-sol/unit/governance/validators/Validators.t.sol index d61e4f8560e..c30627de310 100644 --- a/packages/protocol/test-sol/unit/governance/validators/Validators.t.sol +++ b/packages/protocol/test-sol/unit/governance/validators/Validators.t.sol @@ -4,7 +4,6 @@ pragma experimental ABIEncoderV2; // This test file is in 0.5 although the contract is in 0.8 -import "forge-std/console.sol"; import "celo-foundry/Test.sol"; import "openzeppelin-solidity/contracts/ownership/Ownable.sol"; From d00851155545fe3e6fb51f515a9ca3efcf1a925e Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:43:33 -0500 Subject: [PATCH 13/15] using WhenL2 utils contract --- .../unit/governance/network/Governance.t.sol | 98 +++++++++---------- 1 file changed, 45 insertions(+), 53 deletions(-) diff --git a/packages/protocol/test-sol/unit/governance/network/Governance.t.sol b/packages/protocol/test-sol/unit/governance/network/Governance.t.sol index 168956da476..2ae3310230a 100644 --- a/packages/protocol/test-sol/unit/governance/network/Governance.t.sol +++ b/packages/protocol/test-sol/unit/governance/network/Governance.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.5.13; import "celo-foundry/Test.sol"; import { TestConstants } from "@test-sol/constants.sol"; import { Utils } from "@test-sol/utils.sol"; +import "@test-sol/utils/WhenL2.sol"; import "solidity-bytes-utils/contracts/BytesLib.sol"; import "openzeppelin-solidity/contracts/cryptography/ECDSA.sol"; @@ -14,7 +15,6 @@ import "@celo-contracts/governance/test/MockValidators.sol"; import "@celo-contracts/governance/test/TestTransactions.sol"; import "@celo-contracts/common/Accounts.sol"; import "@celo-contracts/common/Signatures.sol"; -import "@celo-contracts/common/Registry.sol"; import "@celo-contracts/common/FixidityLib.sol"; contract GovernanceMock is Governance(true) { @@ -104,6 +104,7 @@ contract GovernanceTest is Test, TestConstants, Utils { address constant proxyAdminAddress = 0x4200000000000000000000000000000000000018; function setUp() public { + super.setUp(); // Define Accounts accVoter = actor("voter"); accOwner = actor("owner"); @@ -126,10 +127,13 @@ contract GovernanceTest is Test, TestConstants, Utils { // change block.tiemstamp so we're not on timestamp = 0 vm.warp(100 * 60); - + console2.log("###HERE"); setUpContracts(); + console2.log("###HERE1"); setUpVoterAccount(); + console2.log("###HERE2"); setUpProposalStubs(); + console2.log("###HERE3"); } function assertNotEq(uint256 a, uint256 b) internal { @@ -198,18 +202,13 @@ contract GovernanceTest is Test, TestConstants, Utils { function setUpContracts() private { vm.startPrank(accOwner); - Registry registry = new Registry(true); - mockValidators = new MockValidators(); - registry.setAddressFor("Validators", address(mockValidators)); mockLockedGold = new MockLockedGold(); mockLockedGold.setTotalLockedGold(VOTER_GOLD); - registry.setAddressFor("LockedGold", address(mockLockedGold)); accounts = new Accounts(true); accounts.initialize(address(registry)); - registry.setAddressFor("Accounts", address(accounts)); governance = new GovernanceMock(); governance.initialize( @@ -227,6 +226,10 @@ contract GovernanceTest is Test, TestConstants, Utils { baselineQuorumFactor.unwrap() ); vm.stopPrank(); + + registry.setAddressFor("Validators", address(mockValidators)); + registry.setAddressFor("LockedGold", address(mockLockedGold)); + registry.setAddressFor("Accounts", address(accounts)); } function setUpProposalStubs() private { @@ -263,12 +266,7 @@ contract GovernanceTest is Test, TestConstants, Utils { } } -contract TransitionToL2AfterL1 is GovernanceTest { - function setUp() public { - super.setUp(); - _whenL2(); - } -} +contract GovernanceTest_L2 is GovernanceTest, WhenL2 {} contract GovernanceTest_initialize is GovernanceTest { function test_SetsTheOwner() public { @@ -375,7 +373,7 @@ contract GovernanceTest_setApprover is GovernanceTest { } } -contract GovernanceTest_setApprover_L2 is TransitionToL2AfterL1, GovernanceTest_setApprover {} +contract GovernanceTest_setApprover_L2 is GovernanceTest_L2, GovernanceTest_setApprover {} contract GovernanceTest_setMinDeposit is GovernanceTest { uint256 NEW_MINDEPOSIT = 45; @@ -407,7 +405,7 @@ contract GovernanceTest_setMinDeposit is GovernanceTest { } } -contract GovernanceTest_setMinDeposit_L2 is TransitionToL2AfterL1, GovernanceTest_setMinDeposit {} +contract GovernanceTest_setMinDeposit_L2 is GovernanceTest_L2, GovernanceTest_setMinDeposit {} contract GovernanceTest_setConcurrentProposals is GovernanceTest { uint256 NEW_CONCURRENT_PROPOSALS = 45; @@ -446,7 +444,7 @@ contract GovernanceTest_setConcurrentProposals is GovernanceTest { } contract GovernanceTest_setConcurrentProposals_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_setConcurrentProposals {} @@ -485,7 +483,7 @@ contract GovernanceTest_setQueueExpiry is GovernanceTest { } } -contract GovernanceTest_setQueueExpiry_L2 is TransitionToL2AfterL1, GovernanceTest_setQueueExpiry {} +contract GovernanceTest_setQueueExpiry_L2 is GovernanceTest_L2, GovernanceTest_setQueueExpiry {} contract GovernanceTest_setDequeueFrequency is GovernanceTest { event DequeueFrequencySet(uint256 dequeueFrequency); @@ -523,7 +521,7 @@ contract GovernanceTest_setDequeueFrequency is GovernanceTest { } contract GovernanceTest_setDequeueFrequency_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_setDequeueFrequency {} @@ -563,7 +561,7 @@ contract GovernanceTest_setReferendumStageDuration is GovernanceTest { } contract GovernanceTest_setReferendumStageDuration_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_setReferendumStageDuration {} @@ -603,7 +601,7 @@ contract GovernanceTest_setExecutionStageDuration is GovernanceTest { } contract GovernanceTest_setExecutionStageDuration_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_setExecutionStageDuration {} @@ -638,7 +636,7 @@ contract GovernanceTest_setParticipationFloor is GovernanceTest { } contract GovernanceTest_setParticipationFloor_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_setParticipationFloor {} @@ -673,7 +671,7 @@ contract GovernanceTest_setBaselineUpdateFactor is GovernanceTest { } contract GovernanceTest_setBaselineUpdateFactor_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_setBaselineUpdateFactor {} @@ -708,7 +706,7 @@ contract GovernanceTest_setBaselineQuorumFactor is GovernanceTest { } contract GovernanceTest_setBaselineQuorumFactor_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_setBaselineQuorumFactor {} @@ -788,10 +786,7 @@ contract GovernanceTest_setConstitution is GovernanceTest { } } -contract GovernanceTest_setConstitution_L2 is - TransitionToL2AfterL1, - GovernanceTest_setConstitution -{} +contract GovernanceTest_setConstitution_L2 is GovernanceTest_L2, GovernanceTest_setConstitution {} contract GovernanceTest_setSecurityCouncil is GovernanceTest { event SecurityCouncilSet(address indexed council); @@ -839,7 +834,7 @@ contract GovernanceTest_setSecurityCouncil is GovernanceTest { } contract GovernanceTest_setSecurityCouncil_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_setSecurityCouncil {} @@ -874,7 +869,7 @@ contract GovernanceTest_setHotfixExecutionTimeWindow is GovernanceTest { } contract GovernanceTest_setHotfixExecutionTimeWindow_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_setHotfixExecutionTimeWindow {} @@ -1052,7 +1047,7 @@ contract GovernanceTest_propose is GovernanceTest { } } -contract GovernanceTest_propose_L2 is TransitionToL2AfterL1, GovernanceTest_propose {} +contract GovernanceTest_propose_L2 is GovernanceTest_L2, GovernanceTest_propose {} contract GovernanceTest_upvote is GovernanceTest { event ProposalUpvoted(uint256 indexed proposalId, address indexed account, uint256 upvotes); @@ -1253,7 +1248,7 @@ contract GovernanceTest_upvote is GovernanceTest { } } -contract GovernanceTest_upvote_L2 is TransitionToL2AfterL1, GovernanceTest_upvote {} +contract GovernanceTest_upvote_L2 is GovernanceTest_L2, GovernanceTest_upvote {} contract GovernanceTest_revokeUpvote is GovernanceTest { event ProposalExpired(uint256 indexed proposalId); @@ -1344,7 +1339,7 @@ contract GovernanceTest_revokeUpvote is GovernanceTest { } } -contract GovernanceTest_revokeUpvote_L2 is TransitionToL2AfterL1, GovernanceTest_revokeUpvote {} +contract GovernanceTest_revokeUpvote_L2 is GovernanceTest_L2, GovernanceTest_revokeUpvote {} contract GovernanceTest_withdraw is GovernanceTest { address accProposer; @@ -1383,7 +1378,7 @@ contract GovernanceTest_withdraw is GovernanceTest { } } -contract GovernanceTest_withdraw_L2 is TransitionToL2AfterL1, GovernanceTest_withdraw {} +contract GovernanceTest_withdraw_L2 is GovernanceTest_L2, GovernanceTest_withdraw {} contract GovernanceTest_approve is GovernanceTest { uint256 INDEX = 0; // first proposal index @@ -1559,7 +1554,7 @@ contract GovernanceTest_approve is GovernanceTest { } } -contract GovernanceTest_approve_L2 is TransitionToL2AfterL1, GovernanceTest_approve {} +contract GovernanceTest_approve_L2 is GovernanceTest_L2, GovernanceTest_approve {} contract GovernanceTest_revokeVotes is GovernanceTest { uint256 numVoted; @@ -1689,7 +1684,7 @@ contract GovernanceTest_revokeVotes is GovernanceTest { } } -contract GovernanceTest_revokeVotes_L2 is TransitionToL2AfterL1, GovernanceTest_revokeVotes {} +contract GovernanceTest_revokeVotes_L2 is GovernanceTest_L2, GovernanceTest_revokeVotes {} contract GovernanceTest_vote_WhenProposalIsApproved is GovernanceTest { event ProposalVotedV2( @@ -1930,7 +1925,7 @@ contract GovernanceTest_vote_WhenProposalIsApproved is GovernanceTest { } contract GovernanceTest_vote_WhenProposalIsApproved_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_vote_WhenProposalIsApproved {} @@ -2014,7 +2009,7 @@ contract GovernanceTest_vote_WhenProposalIsApprovedAndHaveSigner is GovernanceTe } contract GovernanceTest_vote_WhenProposalIsApprovedAndHaveSigner_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_vote_WhenProposalIsApprovedAndHaveSigner {} @@ -2099,7 +2094,7 @@ contract GovernanceTest_vote_WhenProposalIsNotApproved is GovernanceTest { } contract GovernanceTest_vote_WhenProposalIsNotApproved_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_vote_WhenProposalIsNotApproved {} @@ -2154,7 +2149,7 @@ contract GovernanceTest_vote_WhenVotingOnDifferentProposalWithSameIndex is Gover } contract GovernanceTest_vote_WhenVotingOnDifferentProposalWithSameIndex_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_vote_WhenVotingOnDifferentProposalWithSameIndex {} @@ -2387,7 +2382,7 @@ contract GovernanceTest_vote_PartiallyWhenProposalIsApproved is GovernanceTest { } contract GovernanceTest_vote_PartiallyWhenProposalIsApproved_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_vote_PartiallyWhenProposalIsApproved {} @@ -2506,7 +2501,7 @@ contract GovernanceTest_votePartially_WhenProposalIsApprovedAndHaveSigner is Gov } contract GovernanceTest_votePartially_WhenProposalIsApprovedAndHaveSigner_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_votePartially_WhenProposalIsApprovedAndHaveSigner {} @@ -2591,7 +2586,7 @@ contract GovernanceTest_votePartially_WhenProposalIsNotApproved is GovernanceTes } contract GovernanceTest_votePartially_WhenProposalIsNotApproved_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_votePartially_WhenProposalIsNotApproved {} @@ -2646,7 +2641,7 @@ contract GovernanceTest_votePartially_WhenVotingOnDifferentProposalWithSameIndex } contract GovernanceTest_votePartially_WhenVotingOnDifferentProposalWithSameIndex_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_votePartially_WhenVotingOnDifferentProposalWithSameIndex {} @@ -3081,7 +3076,7 @@ contract GovernanceTest_execute is GovernanceTest { } } -contract GovernanceTest_execute_L2 is TransitionToL2AfterL1, GovernanceTest_execute {} +contract GovernanceTest_execute_L2 is GovernanceTest_L2, GovernanceTest_execute {} contract GovernanceTest_approveHotfix is GovernanceTest { bytes32 constant HOTFIX_HASH = bytes32(uint256(0x123456789)); @@ -3918,7 +3913,7 @@ contract GovernanceTest_isVoting is GovernanceTest { } } -contract GovernanceTest_isVoting_L2 is TransitionToL2AfterL1, GovernanceTest_isVoting {} +contract GovernanceTest_isVoting_L2 is GovernanceTest_L2, GovernanceTest_isVoting {} contract GovernanceTest_isProposalPassing is GovernanceTest { address accSndVoter; @@ -3963,7 +3958,7 @@ contract GovernanceTest_isProposalPassing is GovernanceTest { } contract GovernanceTest_isProposalPassing_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_isProposalPassing {} @@ -4001,7 +3996,7 @@ contract GovernanceTest_dequeueProposalsIfReady is GovernanceTest { } contract GovernanceTest_dequeueProposalsIfReady_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_dequeueProposalsIfReady {} @@ -4137,10 +4132,7 @@ contract GovernanceTest_getProposalStage is GovernanceTest { } } -contract GovernanceTest_getProposalStage_L2 is - TransitionToL2AfterL1, - GovernanceTest_getProposalStage -{} +contract GovernanceTest_getProposalStage_L2 is GovernanceTest_L2, GovernanceTest_getProposalStage {} contract GovernanceTest_getAmountOfGoldUsedForVoting is GovernanceTest { function test_showCorrectNumberOfVotes_whenVotingOn1ConcurrentProposal() public { @@ -4271,7 +4263,7 @@ contract GovernanceTest_getAmountOfGoldUsedForVoting is GovernanceTest { } contract GovernanceTest_getAmountOfGoldUsedForVoting_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_getAmountOfGoldUsedForVoting {} @@ -4437,6 +4429,6 @@ contract GovernanceTest_removeVotesWhenRevokingDelegatedVotes is GovernanceTest } contract GovernanceTest_removeVotesWhenRevokingDelegatedVotes_L2 is - TransitionToL2AfterL1, + GovernanceTest_L2, GovernanceTest_removeVotesWhenRevokingDelegatedVotes {} From b2bff8fae4b704b2e479784e8621ca7ce5a96d72 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:50:17 -0500 Subject: [PATCH 14/15] separate L2 tests for functions that revert on L2 --- .../unit/governance/network/Governance.t.sol | 45 +++++++++++++------ 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/packages/protocol/test-sol/unit/governance/network/Governance.t.sol b/packages/protocol/test-sol/unit/governance/network/Governance.t.sol index 2ae3310230a..c1faa9f0e74 100644 --- a/packages/protocol/test-sol/unit/governance/network/Governance.t.sol +++ b/packages/protocol/test-sol/unit/governance/network/Governance.t.sol @@ -127,13 +127,12 @@ contract GovernanceTest is Test, TestConstants, Utils { // change block.tiemstamp so we're not on timestamp = 0 vm.warp(100 * 60); - console2.log("###HERE"); + setUpContracts(); - console2.log("###HERE1"); + setUpVoterAccount(); - console2.log("###HERE2"); + setUpProposalStubs(); - console2.log("###HERE3"); } function assertNotEq(uint256 a, uint256 b) internal { @@ -3177,10 +3176,12 @@ contract GovernanceTest_approveHotfix_L2 is GovernanceTest { } } -contract GovernanceTest_whitelistHotfix is GovernanceTest { +contract GovernanceTest_whitelistHotfix_setup is GovernanceTest { bytes32 constant HOTFIX_HASH = bytes32(uint256(0x123456789)); event HotfixWhitelisted(bytes32 indexed hash, address whitelister); +} +contract GovernanceTest_whitelistHotfix is GovernanceTest_whitelistHotfix_setup { function test_ShouldWhitelistHotfixByValidator() public { address validator = actor("validator1"); governance.addValidator(validator); @@ -3199,18 +3200,22 @@ contract GovernanceTest_whitelistHotfix is GovernanceTest { vm.prank(validator); governance.whitelistHotfix(HOTFIX_HASH); } +} - function test_Reverts_WhenL2() public { +contract GovernanceTest_whitelistHotfix_L2 is + GovernanceTest_L2, + GovernanceTest_whitelistHotfix_setup +{ + function test_Reverts_WhenCalled() public { address validator = actor("validator1"); governance.addValidator(validator); - _whenL2(); vm.expectRevert("This method is no longer supported in L2."); vm.prank(validator); governance.whitelistHotfix(HOTFIX_HASH); } } -contract GovernanceTest_hotfixWhitelistValidatorTally is GovernanceTest { +contract GovernanceTest_hotfixWhitelistValidatorTally_setup is GovernanceTest { bytes32 constant HOTFIX_HASH = bytes32(uint256(0x123456789)); address[] validators; @@ -3233,7 +3238,11 @@ contract GovernanceTest_hotfixWhitelistValidatorTally is GovernanceTest { signers.push(signer); } } +} +contract GovernanceTest_hotfixWhitelistValidatorTally is + GovernanceTest_hotfixWhitelistValidatorTally_setup +{ function test_countValidatorAccountsThatHaveWhitelisted() public { for (uint256 i = 0; i < 3; i++) { vm.prank(validators[i]); @@ -3275,17 +3284,21 @@ contract GovernanceTest_hotfixWhitelistValidatorTally is GovernanceTest { assertEq(governance.hotfixWhitelistValidatorTally(HOTFIX_HASH), 3); } +} - function test_Reverts_WhenL2() public { +contract GovernanceTest_hotfixWhitelistValidatorTally_L2 is + GovernanceTest_L2, + GovernanceTest_hotfixWhitelistValidatorTally_setup +{ + function test_Reverts_WhenCalled() public { address validator = actor("validator1"); governance.addValidator(validator); - _whenL2(); vm.expectRevert("This method is no longer supported in L2."); governance.hotfixWhitelistValidatorTally(HOTFIX_HASH); } } -contract GovernanceTest_isHotfixPassing is GovernanceTest { +contract GovernanceTest_isHotfixPassing_setup is GovernanceTest { bytes32 constant HOTFIX_HASH = bytes32(uint256(0x123456789)); address validator1; address validator2; @@ -3302,7 +3315,9 @@ contract GovernanceTest_isHotfixPassing is GovernanceTest { vm.prank(validator2); accounts.createAccount(); } +} +contract GovernanceTest_isHotfixPassing is GovernanceTest_isHotfixPassing_setup { function test_returnFalseWhenHotfixHasNotBeenWhitelisted() public { assertFalse(governance.isHotfixPassing(HOTFIX_HASH)); } @@ -3320,9 +3335,13 @@ contract GovernanceTest_isHotfixPassing is GovernanceTest { governance.whitelistHotfix(HOTFIX_HASH); assertTrue(governance.isHotfixPassing(HOTFIX_HASH)); } +} - function test_Reverts_WhenL2() public { - _whenL2(); +contract GovernanceTest_isHotfixPassing_L2 is + GovernanceTest_L2, + GovernanceTest_isHotfixPassing_setup +{ + function test_Reverts_WhenCalled() public { vm.expectRevert("This method is no longer supported in L2."); governance.isHotfixPassing(HOTFIX_HASH); } From 88fb9ae5b1941c81325db5d59b757b1087f33f07 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Fri, 15 Nov 2024 11:17:08 -0500 Subject: [PATCH 15/15] PR feedback --- .../unit/governance/network/Governance.t.sol | 40 ++++++------------- 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/packages/protocol/test-sol/unit/governance/network/Governance.t.sol b/packages/protocol/test-sol/unit/governance/network/Governance.t.sol index c1faa9f0e74..efc69e0d0cc 100644 --- a/packages/protocol/test-sol/unit/governance/network/Governance.t.sol +++ b/packages/protocol/test-sol/unit/governance/network/Governance.t.sol @@ -3516,7 +3516,7 @@ contract GovernanceTest_prepareHotfix_L2 is GovernanceTest { } } -contract GovernanceTest_resetHotfix is GovernanceTest { +contract GovernanceTest_resetHotfix_setup is GovernanceTest { bytes32 constant HOTFIX_HASH = bytes32(uint256(0x123456789)); bytes32 constant SALT = 0x657ed9d64e84fa3d1af43b3a307db22aba2d90a158015df1c588c02e24ca08f0; bytes32 hotfixHash; @@ -3525,12 +3525,6 @@ contract GovernanceTest_resetHotfix is GovernanceTest { function setUp() public { super.setUp(); - - validator1 = actor("validator1"); - governance.addValidator(validator1); - vm.prank(validator1); - accounts.createAccount(); - vm.prank(accOwner); governance.setSecurityCouncil(accCouncil); @@ -3542,6 +3536,17 @@ contract GovernanceTest_resetHotfix is GovernanceTest { SALT ); } +} + +contract GovernanceTest_resetHotfix is GovernanceTest_resetHotfix_setup { + function setUp() public { + super.setUp(); + + validator1 = actor("validator1"); + governance.addValidator(validator1); + vm.prank(validator1); + accounts.createAccount(); + } function test_Reverts_whenCalledOnL1() public { vm.prank(accOwner); @@ -3569,26 +3574,7 @@ contract GovernanceTest_resetHotfix is GovernanceTest { } } -contract GovernanceTest_resetHotfix_L2 is GovernanceTest { - bytes32 constant HOTFIX_HASH = bytes32(uint256(0x123456789)); - bytes32 constant SALT = 0x657ed9d64e84fa3d1af43b3a307db22aba2d90a158015df1c588c02e24ca08f0; - bytes32 hotfixHash; - event HotfixRecordReset(bytes32 indexed hash); - - function setUp() public { - super.setUp(); - _whenL2(); - vm.prank(accOwner); - governance.setSecurityCouncil(accCouncil); - - hotfixHash = governance.getHotfixHash( - okProp.values, - okProp.destinations, - okProp.data, - okProp.dataLengths, - SALT - ); - } +contract GovernanceTest_resetHotfix_L2 is GovernanceTest_L2, GovernanceTest_resetHotfix_setup { function test_ShouldResetHotfixRecordWhenExecutionTimeLimitHasPassed() public { vm.prank(accOwner); governance.setHotfixExecutionTimeWindow(DAY);